Closed Bug 1450344 Opened 2 years ago Closed 2 years ago

<select> prompts don't work inside e10s iframes

Categories

(GeckoView :: General, enhancement, P1)

enhancement

Tracking

(firefox61 fixed)

RESOLVED FIXED
mozilla61
Tracking Status
firefox61 --- fixed

People

(Reporter: jchen, Assigned: jchen)

Details

(Whiteboard: [geckoview:klar])

Attachments

(3 files)

Clicking on <select> does not show a prompt when it's inside an iframe under e10s
Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

https://reviewboard.mozilla.org/r/233128/#review238580


Code analysis found 1 defect in this patch:
 - 1 defect found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:231
(Diff revision 1)
> +        let mm = this.getContentFrameMessageManager(aWin.top || aWin);
> +        return mm && EventDispatcher.forMessageManager(mm);
> +      }
>        let win = this.getChromeWindow(aWin.top || aWin);
> -      let dispatcher = win.WindowEventDispatcher || EventDispatcher.for(win);
> -      if (!win.closed && dispatcher) {
> +      if (!win.closed) {
> +        return win.WindowEventDispatcher || EventDispatcher.for(win)

Error: Missing semicolon. [eslint: semi]
Comment on attachment 8964403 [details]
Bug 1450344 - 1. Use frame script to handle input prompts;

https://reviewboard.mozilla.org/r/233126/#review239234
Attachment #8964403 - Flags: review?(esawin) → review+
Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

https://reviewboard.mozilla.org/r/233128/#review239238

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm:229
(Diff revision 1)
>      try {
> +      if (!this.IS_PARENT_PROCESS) {
> +        let mm = this.getContentFrameMessageManager(aWin.top || aWin);
> +        return mm && EventDispatcher.forMessageManager(mm);
> +      }
>        let win = this.getChromeWindow(aWin.top || aWin);

const

::: mobile/android/modules/geckoview/GeckoViewUtils.jsm
(Diff revision 1)
> -      let dispatcher = win.WindowEventDispatcher || EventDispatcher.for(win);
> -      if (!win.closed && dispatcher) {
> +      if (!win.closed) {
> +        return win.WindowEventDispatcher || EventDispatcher.for(win)
> -        return dispatcher;
>        }
>      } catch (e) {
> -      return null;

Don't we want to log here?
Attachment #8964404 - Flags: review?(esawin) → review+
Comment on attachment 8964405 [details]
Bug 1450344 - 3. Use EventDispatcher directly for child process prompts;

https://reviewboard.mozilla.org/r/233130/#review239240
Attachment #8964405 - Flags: review?(esawin) → review+
Comment on attachment 8964404 [details]
Bug 1450344 - 2. Make getDispatcherForWindow work in child process;

https://reviewboard.mozilla.org/r/233128/#review239238

> Don't we want to log here?

That's up to the caller. It may expect a null result.
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5cfb9e997161
1. Use frame script to handle input prompts; r=esawin
https://hg.mozilla.org/integration/autoland/rev/f69c0c61aa09
2. Make getDispatcherForWindow work in child process; r=esawin
https://hg.mozilla.org/integration/autoland/rev/66f0126bec69
3. Use EventDispatcher directly for child process prompts; r=esawin
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 61 → mozilla61
You need to log in before you can comment on or make changes to this bug.