Closed Bug 674321 Opened 9 years ago Closed 8 years ago

port docshell_helpers.js to SpecialPowers

Categories

(Testing :: Mochitest, defect)

x86
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla10

People

(Reporter: jmaher, Assigned: jmaher)

References

Details

(Whiteboard: [specialpowers])

Attachments

(1 file, 2 obsolete files)

simple patch to fix the enablePrivilege calls in docshell_helpers.js
can you give this a standard review.  Should be straightforward.
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #548550 - Flags: review?(jgriffin)
Comment on attachment 548550 [details] [diff] [review]
port docshell_helpers.js to SpecialPowers (1.0)

Looks good, thanks!
Attachment #548550 - Flags: review?(jgriffin) → review+
Whiteboard: [specialpowers] → [specialpowers][inbound]
http://hg.mozilla.org/mozilla-central/rev/de3002d94566
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
SpecialPowers is not defined in a bunch of places where docshell_helpers is used.  As far as I can tell, this change just disabled a bunch of tests (including I believe all the bfcache tests) by making them throw when trying to get preferences...
Filed bug 688681 on backing this out.
Backed out to restore test coverage.

As far as doing this "right", seems like we should just add helper functions to docshell_helpers that use SpecialPowers if present and direct XPCOM if not (so in chrome).

Or if the latter is undesirable and we want to go through SpecialPowers in Chrome as well, we should think about how to best do that.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
the original patch was backed out because specialpowers was not available to all chrome tests.  Now that bug 676274 has landed, we have specialpowers in all chrome tests.  I verified the problematic tests which caused us to back this out are working with this.
Attachment #548550 - Attachment is obsolete: true
Attachment #566178 - Flags: review?(ted.mielczarek)
Comment on attachment 566178 [details] [diff] [review]
port docshell_helpers.js to SpecialPowers (2.0)

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

::: docshell/test/chrome/docshell_helpers.js
@@ +316,5 @@
>    
>    // If the test changed the value of max_total_viewers via a call to
>    // enableBFCache(), then restore it now.
>    if (typeof(gOrigMaxTotalViewers) != "undefined") {
> +    SpecialPowers.setIntPref("browser.sessionhistory.max_total_viewers",

Does this actually work post-bug 621363 landing?

@@ +396,5 @@
>   *   enable: if true, set max_total_viewers to -1 (the default); if false, set 
>   *           to 0 (disabled), if a number, set it to that specific number
>   */
>  function enableBFCache(enable) {
> +  var prefs = SpecialPowers;

This is weird, I'd rather you just explicitly change the lines that use prefs to refer to SpecialPowers.
Attachment #566178 - Flags: review?(ted.mielczarek) → review-
Whiteboard: [specialpowers][inbound] → [specialpowers]
after further review, it seems that docshell_helpers.js is only used by chrome/browser-chrome tests, so we don't need this in mochitest-plain.
Attachment #566178 - Attachment is obsolete: true
Attachment #567431 - Flags: review?(ted.mielczarek)
Attachment #567431 - Flags: review?(ted.mielczarek) → review+
https://hg.mozilla.org/mozilla-central/rev/2c4eccfeceed
Status: REOPENED → RESOLVED
Closed: 9 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: mozilla8 → mozilla10
You need to log in before you can comment on or make changes to this bug.