Closed Bug 1069757 Opened 5 years ago Closed 5 years ago

browser_urlbarCopying.js fails with e10s

Categories

(Firefox :: General, defect)

defect
Not set
Points:
1

Tracking

()

RESOLVED FIXED
Firefox 36
Iteration:
36.2
Tracking Status
e10s + ---

People

(Reporter: markh, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

browser_urlbarCopying.js is currently disabled due to its use of the clipboard (bug 932651) - however, once that is fixed there is one remaining problem.  The test waits for a "load" event on the child <browser>, but this fires too early for the test - the currentURI hasn't been updated by that point.

This patch adds a web-progress listener and performs the check once onLocationChange fires.

Asking dao for review as he has touched this file relatively recently...
Attachment #8492004 - Flags: review?(dao)
Comment on attachment 8492004 [details] [diff] [review]
0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch

Why do we get the load event before onLocationChange? This seems backwards and like a potential source for confusion for production code...
Attachment #8492004 - Flags: review?(dao)
(In reply to Dão Gottwald [:dao] from comment #1)
> Comment on attachment 8492004 [details] [diff] [review]
> 0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch
> 
> Why do we get the load event before onLocationChange? This seems backwards
> and like a potential source for confusion for production code...

The load event is on the browser itself.  This load event isn't coordinated with events in the child process - ie, the load event on the content-process document could theoretically happen before or after the browser element's load event in the parent.

Does that clarify?
Flags: needinfo?(dao)
The load event on the browser is still about the content being loaded, which means it originally comes from the content process. Right?
Flags: needinfo?(dao)
I agree with dao, the load event should be fine for this. But it is possible there are multiple loads here and we're not waiting for the right one? That's a common issue when loading things in a new tab, for example.
Comment on attachment 8492004 [details] [diff] [review]
0001-Bug-XXXXXXX-fix-urlbarCopying-test-under-e10s.-r-dao.patch

OK, no problems, we'll just leave this disabled in e10s for the time being.
Attachment #8492004 - Attachment is obsolete: true
Locally, this test runs fine in e10s at this point. Try push: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=ade58da9cb1f
Attachment #8517059 - Flags: review?(mhammond)
Blocks: 1080801
Comment on attachment 8517059 [details] [diff] [review]
re-enable browser_urlbarCopying.js in e10s mode,

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

awesome!
Attachment #8517059 - Flags: review?(mhammond) → review+
remote:   https://hg.mozilla.org/integration/fx-team/rev/80a1da90e093
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 36.2
Points: --- → 1
Whiteboard: [fixed-in-fx-team]
Flags: qe-verify?
Flags: firefox-backlog+
Flags: qe-verify? → qe-verify-
https://hg.mozilla.org/mozilla-central/rev/80a1da90e093
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
You need to log in before you can comment on or make changes to this bug.