Last Comment Bug 653461 - avoid uses of enablePrivilege in style system mochitests
: avoid uses of enablePrivilege in style system mochitests
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: P4 normal (vote)
: mozilla6
Assigned To: David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-28 09:36 PDT by David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
Modified: 2012-09-07 08:46 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch 1: fix use of prefs API (9.14 KB, patch)
2011-04-28 09:37 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 2: convert uses of DOMWindowUtils (6.98 KB, patch)
2011-04-28 09:37 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
patch 3: convert use of zoom (1.47 KB, patch)
2011-04-28 09:38 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review
use SpecialPowers.wrap() for the last case (1.19 KB, patch)
2012-09-06 18:54 PDT, David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch)
bzbarsky: review+
Details | Diff | Review

Description David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-28 09:36:11 PDT
I have patches to remove all but one of the uses of enablePrivilege in the layout/style/test/ mochitests.

The one remaining use is in test_bug405818.html, which I'm not immediately sure what to do about.
Comment 1 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-28 09:37:07 PDT
Created attachment 528879 [details] [diff] [review]
patch 1: fix use of prefs API
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-28 09:37:55 PDT
Created attachment 528881 [details] [diff] [review]
patch 2: convert uses of DOMWindowUtils
Comment 3 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-28 09:38:25 PDT
Created attachment 528882 [details] [diff] [review]
patch 3: convert use of zoom
Comment 4 Joel Maher (:jmaher) 2011-04-28 09:43:54 PDT
Comment on attachment 528881 [details] [diff] [review]
patch 2: convert uses of DOMWindowUtils

Review of attachment 528881 [details] [diff] [review]:

::: layout/style/test/test_pointer-events.html
@@ +70,5 @@
                               modifiers,           // long
                               ignoreWindowBounds)  // boolean
 {
+  SpecialPowers.DOMWindowUtils.sendMouseEvent(type, x, y, button, clickCount,
+                                              modifiers, ignoreWindowBounds);

Just curious why this is using sendMouseEvent when we have many synthesizeMouse* functions in EventUtils.js?  I sort of prefer this simpler approach.
Comment 5 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-28 09:48:39 PDT
Not sure; I didn't write it, and I'd rather not change it without doing more retesting that it's still testing the original bug.

That said, I find the EventUtils stuff pretty magical, and prefer this more explicit approach.
Comment 6 Boris Zbarsky [:bz] 2011-04-28 12:11:50 PDT
This caller is passing true for ignoreWindowBounds, which is not something the synthesizeMouse stuff in EventUtils does.  That my matter in this case.  Or not.  It would take some looking at this test to make sure.
Comment 7 Boris Zbarsky [:bz] 2011-04-28 12:13:50 PDT
Comment on attachment 528879 [details] [diff] [review]
patch 1: fix use of prefs API

r=me
Comment 8 Boris Zbarsky [:bz] 2011-04-28 12:14:41 PDT
Comment on attachment 528881 [details] [diff] [review]
patch 2: convert uses of DOMWindowUtils

r=me
Comment 9 Boris Zbarsky [:bz] 2011-04-28 12:16:29 PDT
Comment on attachment 528882 [details] [diff] [review]
patch 3: convert use of zoom

r=me
Comment 10 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-04-30 15:29:20 PDT
https://hg.mozilla.org/mozilla-central/rev/2228e6343c47
https://hg.mozilla.org/mozilla-central/rev/442fe6006bbb
https://hg.mozilla.org/mozilla-central/rev/00d9bc4a9b9c

I'll call this bug fixed despite there being one left (see comment 0).
Comment 11 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-09-06 18:54:09 PDT
Created attachment 659077 [details] [diff] [review]
use SpecialPowers.wrap() for the last case

Well, SpecialPowers.wrap() makes the last case easy (though I wonder if it's an overused tool), assuming this looks reasonable.
Comment 12 Boris Zbarsky [:bz] 2012-09-06 20:45:00 PDT
Comment on attachment 659077 [details] [diff] [review]
use SpecialPowers.wrap() for the last case

r=me
Comment 13 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2012-09-06 21:17:40 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/9a64e93daf5c
Comment 14 Ed Morley [:emorley] 2012-09-07 08:46:16 PDT
https://hg.mozilla.org/mozilla-central/rev/9a64e93daf5c

Note You need to log in before you can comment on or make changes to this bug.