Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change initElements parameter from WebDriver to SearchContext #9549

Closed
Jiwari opened this issue Jun 4, 2021 · 3 comments
Closed

Change initElements parameter from WebDriver to SearchContext #9549

Jiwari opened this issue Jun 4, 2021 · 3 comments

Comments

@Jiwari
Copy link
Contributor

Jiwari commented Jun 4, 2021

🚀 Feature Proposal

Update PageFactory methods signature from WebDriver to SearchContext.

Motivation

Allow elements to be passed as an argument to PageFactory#initElements methods and be used as the SearchContext.

My objective here is to solve issues related to shadow doms and elements lookup.

I currently have this changed implemented on my project which has a lot of shadow DOMs on the page, and being able to provide the shadow element to initElements allows me to use PageFactory with the shadow element as the search context, providing WebDriver would never work as the elements are "hidden" inside the shadow DOM.

Example

Only 3 methods need to be updated.

Currently:

  public static void initElements(WebDriver driver, Object page) {
    initElements(new DefaultElementLocatorFactory(driver), page);
  }

  public static <T> T initElements(WebDriver driver, Class<T> pageClassToProxy) {
    T page = instantiatePage(driver, pageClassToProxy);
    initElements(driver, page);
    return page;
  }

  private static <T> T instantiatePage(WebDriver driver, Class<T> pageClassToProxy) {
    try {
      try {
        Constructor<T> constructor = pageClassToProxy.getConstructor(WebDriver.class);
        return constructor.newInstance(driver);
      } catch (NoSuchMethodException e) {
        return pageClassToProxy.getDeclaredConstructor().newInstance();
      }
    } catch (ReflectiveOperationException e) {
      throw new RuntimeException(e);
    }
  }

With changes:

  public static void initElements(SearchContext searchContext, Object page) {
    initElements(new DefaultElementLocatorFactory(searchContext), page);
  }

  public static <T> T initElements(SearchContextsearchContext, Class<T> pageClassToProxy) {
    T page = instantiatePage(searchContext, pageClassToProxy);
    initElements(searchContext, page);
    return page;
  }

  private static <T> T instantiatePage(SearchContext searchContext, Class<T> pageClassToProxy) {
    try {
      try {
        Constructor<T> constructor = pageClassToProxy.getConstructor(WebDriver.class);
        return constructor.newInstance(searchContext);
      } catch (NoSuchMethodException e) {
        return pageClassToProxy.getDeclaredConstructor().newInstance();
      }
    } catch (ReflectiveOperationException e) {
      throw new RuntimeException(e);
    }
  }

No further changes would be needed as upstream classes already use SearchContext as type instead of WebDriver

My current concern is about instantiatePage change. Currently Selenium tries to instantiate the page with 2 different constructors (WebDriver arg and parameterless), I am wondering if there should be a third for SearchContext between the two existing ones, and if it would be useful.

@ghost ghost added the needs-triaging label Jun 4, 2021
@Jiwari
Copy link
Contributor Author

Jiwari commented Jun 4, 2021

I will work on a branch with these changes later this weekend

@diemol
Copy link
Member

diemol commented Jun 4, 2021

I would recommend to avoid creating a PR before having a chat on the approach, otherwise the PR might get rejected if it does not align with the implementation idea that Java maintainers have (whatever the idea is). Shadow DOM landed in the W3C spec a while ago and we are waiting for browser vendors to support it (also, follow #5869). In the meantime, the bindings maintainers are starting to implement the endpoints for it.

If you'd like to contribute and have a chat, please join us in our Slack/IRC channel, maintainers normally hangout in the #selenium-tlc channel, and probably you can discuss your approach with @shs96c and others.

@diemol
Copy link
Member

diemol commented Jun 18, 2021

Closed via #9557

@diemol diemol closed this as completed Jun 18, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Sep 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants