The default bug view has changed. See this FAQ.

Make site-specific zoom check asynchronous

RESOLVED FIXED in Firefox 3.7a4

Status

()

Firefox
General
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: rflint, Assigned: rflint)

Tracking

({perf})

Trunk
Firefox 3.7a4
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -
in-litmus -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [tsnap])

Attachments

(1 attachment, 2 obsolete attachments)

Created attachment 423215 [details] [diff] [review]
WIP
Created attachment 428858 [details] [diff] [review]
Patch

Mostly test changes - I've removed the print preview tests because getting them working is non-trivial and I plan to rip out in-window previewing anyway.
Attachment #423215 - Attachment is obsolete: true
Attachment #428858 - Flags: review?(myk)
The last time browser_bug386835.js caught a print preview regression was in bug 544018 comment 23 :/
Comment on attachment 428858 [details] [diff] [review]
Patch

Sorry about the delay reviewing this!


>+    // Avoid the cps roundtrip and apply the default/global setting.

Nit: setting -> pref!


>+    var self = this;
>+    this._cps.getPref(aURI, this.name, function(aResult) {
>+      // Check that we're still where we expect to be in case this took a while.
>+      let isSaneURI = (aBrowser && aBrowser.currentURI) ?
>+        aURI.equals(aBrowser.currentURI) : false;
>+      if (!aBrowser || isSaneURI)
>+        self._applyPrefToSetting(aResult, aBrowser);
>+      else
>+        self._applyPrefToSetting();
>+    });

If the browser's current URI is different than the one for which we got the pref, is it possible that the application of the default/global pref will race the application of the site-specific pref for the new URL?

And is it necessary to apply the default/global pref at all if the URLs differ?  Presumably onLocationChange will get called again for the new URL, and it'll apply either the site-specific pref for that URL or the default/global pref.
Created attachment 433472 [details] [diff] [review]
Patch v2

(In reply to comment #3)
> If the browser's current URI is different than the one for which we got the
> pref, is it possible that the application of the default/global pref will race
> the application of the site-specific pref for the new URL?
> 
> And is it necessary to apply the default/global pref at all if the URLs differ?

It can indeed race, and no, it's not necessary. I think I added that while debugging something and simply forgot to remove it.
Attachment #428858 - Attachment is obsolete: true
Attachment #433472 - Flags: review?(myk)
Attachment #428858 - Flags: review?(myk)
Comment on attachment 433472 [details] [diff] [review]
Patch v2

>diff --git a/browser/base/content/browser-fullZoom.js b/browser/base/content/browser-fullZoom.js
>-    var browser = aBrowser || gBrowser.selectedBrowser;
>+    var browser = aBrowser || (gBrowser && gBrowser.selectedBrowser);

Heh, funky, but I guess it works.

r=myk!
Attachment #433472 - Flags: review?(myk) → review+
http://hg.mozilla.org/mozilla-central/rev/a2f7186e4379
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Flags: in-testsuite-
Flags: in-litmus-
Resolution: --- → FIXED
Target Milestone: Firefox 3.7a1 → Firefox 3.7a4

Comment 7

7 years ago
Zoomed page does not flicker after switching back tab.

Comment 8

7 years ago
Oops,Sorry, I have transmitted a message by mistake.

Updated

7 years ago
Depends on: 555211

Updated

7 years ago
Depends on: 555224

Comment 9

7 years ago
Is variable name isSaneURI in the patch a spelling mistake of isSameURI?
Depends on: 559991

Updated

7 years ago
Depends on: 559992
You need to log in before you can comment on or make changes to this bug.