1099154 - Fix browser_backButtonFitts.js to work in e10s mode

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[:Gijs (he/him)]

This test adds an event listener to the content window and then gets stuck after a new tab has been loaded.

Comment 1

[Neil Deakin]

Attachment: browser_backButtonFitts

Success on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ad8793ebfea1

Comment 2

[:Gijs (he/him)]

Comment on attachment 8595680 [details] [diff] [review]
browser_backButtonFitts

Review of attachment 8595680 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/base/content/test/general/browser_backButtonFitts.js
@@ +11,5 @@
>  
> +    content.addEventListener("popstate", function onPopState() {
> +      content.removeEventListener("popstate", onPopState, false);
> +      sendAsyncMessage("Test:PopStateOccurred", { location: content.document.location.href });
> +    }, false);

Pretty sure you could return a promise here that you'd resolve from onPopState, and then the ContentTask.spawn call will yield you the thing you resolve the promise with. Then you don't need to do your own message passing here and below.

Comment 3

[Neil Deakin]

> Pretty sure you could return a promise here that you'd resolve from
> onPopState, and then the ContentTask.spawn call will yield you the thing you
> resolve the promise with. Then you don't need to do your own message passing
> here and below.

I don't think I follow here. The popstate event doesn't happen until the mouse event occurs later on, long after the ContentTask.spawn has already finished.

Comment 4

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #3)
> > Pretty sure you could return a promise here that you'd resolve from
> > onPopState, and then the ContentTask.spawn call will yield you the thing you
> > resolve the promise with. Then you don't need to do your own message passing
> > here and below.
> 
> I don't think I follow here. The popstate event doesn't happen until the
> mouse event occurs later on, long after the ContentTask.spawn has already
> finished.

let promise = ContentTask.spawn(gBrowser.selectedBrowser, {}, function* () {
  content.history.pushState("page2", "page2", "page2");
  return new Promise((resolve, reject) => {
    content.addEventListener("popstate", function onPopState() {
      content.removeEventListener("popstate", onPopState, false);
      resolve(content.document.location.href);
    }, false);
  });
});

// trigger click etc.

let uri = yield promise;

is(uri, ...);


See e.g. http://mxr.mozilla.org/mozilla-central/source/dom/html/test/browser_DOMDocElementInserted.js#6 for another test that uses this structure.

Comment 5

[Neil Deakin]

> let promise = ContentTask.spawn(gBrowser.selectedBrowser, {}, function* () {
>   content.history.pushState("page2", "page2", "page2");
>   return new Promise((resolve, reject) => {
>     content.addEventListener("popstate", function onPopState() {
>       content.removeEventListener("popstate", onPopState, false);
>       resolve(content.document.location.href);
>     }, false);
>   });
> });
> 
>
> // trigger click etc.

Yes, but I want the call to pushState to happen before the mouse click.

I used a message here only to avoid having two separate ContentTasks.

Comment 6

[:Gijs (he/him)]

(In reply to Neil Deakin from comment #5)
> > let promise = ContentTask.spawn(gBrowser.selectedBrowser, {}, function* () {
> >   content.history.pushState("page2", "page2", "page2");
> >   return new Promise((resolve, reject) => {
> >     content.addEventListener("popstate", function onPopState() {
> >       content.removeEventListener("popstate", onPopState, false);
> >       resolve(content.document.location.href);
> >     }, false);
> >   });
> > });
> > 
> >
> > // trigger click etc.
> 
> Yes, but I want the call to pushState to happen before the mouse click.
> 
> I used a message here only to avoid having two separate ContentTasks.

Huh, I hadn't thought of that. Fair point. I would actually prefer the two-contentTask solution, I guess? With a comment, I suppose, about needing it to have happened before the click.

Comment 7

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/41ca098906eb

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/41ca098906eb