[e10s] getClipboardData in specialpowersAPI not e10s friendly

VERIFIED FIXED in mozilla35

Status

Testing
Mochitest
VERIFIED FIXED
4 years ago
3 years ago

People

(Reporter: markh, Assigned: markh, Mentored)

Tracking

(Blocks: 2 bugs)

unspecified
mozilla35
Points:
1
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(e10s+)

Details

(Whiteboard: [good first bug][lang=js] [qa-])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
getClipboardData does 'this._getDocShell(content.window)' - this doesn't work in e10s for 2 reasons:

* content.window may not exist yet - it is setup asynchronously - eg, browser_bug556061.js hits this.

* Even if content.window was setup, there's no docShell for it (the docShell is in the content process)
tracking-e10s: --- → +
Blocks: 984139
Whiteboard: [good-first-bug][lang=js][mentor=mrbkap]

Updated

4 years ago
Whiteboard: [good-first-bug][lang=js][mentor=mrbkap] → [good first bug][lang=js][mentor=mrbkap]
Mentor: mrbkap@mozilla.com
Whiteboard: [good first bug][lang=js][mentor=mrbkap] → [good first bug][lang=js]
(Assignee)

Updated

4 years ago
Blocks: 1069757
(Assignee)

Comment 1

4 years ago
Created attachment 8492007 [details] [diff] [review]
0002-Bug-932651-getClipboardData-in-specialpowersAPI-uses.patch

Now that the password manager is e10s friendly, attempting to enable those tests hit this clipboard problem.  A simple fix to the clipboard problem is to use the chrome window rather than the content window, and doing this allows all the password manager tests to pass under e10s, as well as the 2 tests disabled due to this bug (well, nearly - one of those other tests now hits a different issue, which I've recorded in browser.ini)
Attachment #8492007 - Flags: review?(ted)
Comment on attachment 8492007 [details] [diff] [review]
0002-Bug-932651-getClipboardData-in-specialpowersAPI-uses.patch

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

I can't say I understand the mechanics of why it works, but it's fine with me.
Attachment #8492007 - Flags: review?(ted) → review+
(Assignee)

Comment 3

4 years ago
I ended up pushing a slightly modified version of the patch due to failures on try - the relevant part of SpecialPowers pushed was:

-    xferable.init(this._getDocShell(content.window)
+    // in e10s b-c tests |content.window| is null whereas |window| works fine.
+    // for some non-e10s mochi tests, |window| is null whereas |content.window|
+    // works fine.  So we take whatever is non-null!
+    xferable.init(this._getDocShell(content.window || window)

Which I figured was close enough to the original intent I didn't re-request review.  Try at https://tbpl.mozilla.org/?tree=Try&rev=cd2b8f931de2

https://hg.mozilla.org/integration/fx-team/rev/5be05e3aa09c
Assignee: nobody → mhammond
Status: NEW → ASSIGNED
Points: --- → 1
Flags: firefox-backlog+

Updated

4 years ago
Iteration: --- → 35.2
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js] [qa?]
https://hg.mozilla.org/mozilla-central/rev/5be05e3aa09c
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
(Assignee)

Updated

3 years ago
Status: RESOLVED → VERIFIED
Whiteboard: [good first bug][lang=js] [qa?] → [good first bug][lang=js] [qa-]
You need to log in before you can comment on or make changes to this bug.