Closed
Bug 988334
Opened 11 years ago
Closed 11 years ago
Stop reusing SpecialPowers for different windows
Categories
(Testing :: General, defect)
Testing
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla31
People
(Reporter: peterv, Assigned: peterv)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
SpecialPowers has a DOMWindowUtils property, which is a per-window object. attachSpecialPowersToWindow sometimes reuses the SpecialPowers of the .parent, meaning that we end up with SpecialPowers.DOMWindowUtils from the wrong window. Some DOMWindowUtils properties/methods use the window object that the DOMWindowUtils is related to, leading to weird behaviour/breakage. I remove the code to reuse SpecialPowers and pushed to try. The time to run tests didn't seem impacted at all, so I think we should stop reusing SpecialPowers objects.
Attachment #8397090 -
Flags: review?(jmaher)
Comment 1•11 years ago
|
||
Thanks, I don't think any of us really understand what we're doing with the DOM bits, what you see is just fumbling in the dark trying to get things working across the various Mochitest types.
Comment 2•11 years ago
|
||
Comment on attachment 8397090 [details] [diff] [review]
v1
Review of attachment 8397090 [details] [diff] [review]:
-----------------------------------------------------------------
r=me if this passes on try for all mochitest types on all platforms
::: testing/specialpowers/content/specialpowers.js
@@ +103,5 @@
> try {
> if ((aWindow !== null) &&
> (aWindow !== undefined) &&
> (aWindow.wrappedJSObject) &&
> + !(aWindow.wrappedJSObject.SpecialPowers)) {
while I like this simplification, does this work in all cases?
Attachment #8397090 -
Flags: review?(jmaher) → review+
Assignee | ||
Comment 3•11 years ago
|
||
Comment on attachment 8397090 [details] [diff] [review]
v1
Review of attachment 8397090 [details] [diff] [review]:
-----------------------------------------------------------------
::: testing/specialpowers/content/specialpowers.js
@@ +103,5 @@
> try {
> if ((aWindow !== null) &&
> (aWindow !== undefined) &&
> (aWindow.wrappedJSObject) &&
> + !(aWindow.wrappedJSObject.SpecialPowers)) {
This is just the existing code that used to be in the |else if|.
Assignee | ||
Comment 4•11 years ago
|
||
Comment 5•11 years ago
|
||
3 bugs backed out for failures in test_bug470212.html:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=8059895497c1&jobname=mochitest-9
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=4fc31f234001
Assignee | ||
Comment 6•11 years ago
|
||
Relanded with a fix for the orange. Because we now use the SpecialPowers related to the window and not its parent, the test's mouse event were sent at slightly different coordinates (which seems to only affect the "B2G ICS Emulator Opt" build :-/). I adjusted the test so the coordinates are closer to the original values.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fb101fc5976
Comment 7•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•