Port |Bug 575830 - Image zoom (Page zoom) is reset when I switch tabs| to SeaMonkey

RESOLVED FIXED in seamonkey2.12


Tabbed Browser
6 years ago
5 years ago


(Reporter: mcsmurf, Assigned: mcsmurf)


Windows XP

Firefox Tracking Flags

(Not tracked)



(1 attachment, 2 obsolete attachments)



6 years ago
Bug 575830 fixed the problem that Firefox forgot the current zoom level in a tab when switching tabs. SeaMonkey has the same problem so that patch should be ported to SeaMonkey.

Comment 1

6 years ago
Created attachment 591902 [details] [diff] [review]

Also includes fix from Bug 719271.
Neil: Can you check if my usage of content.browser is correct? The Firefox code uses both browser.contentDocument.mozSyntheticDocument and content.document.mozSyntheticDocument in the same file. I changed that code to content.document... Is that correct?
Assignee: nobody → bugzilla
Attachment #591902 - Flags: review?(neil)

Comment 2

6 years ago
Comment on attachment 591902 [details] [diff] [review]

Not ready for review yet.
Attachment #591902 - Flags: review?(neil)

Comment 3

6 years ago
Comment on attachment 591902 [details] [diff] [review]

>+    if (!aIsTabSwitch && content.document.mozSyntheticDocument) {
No, this needs to use aBrowser.contentDocument for consistency. (As far as I can tell, aBrowser is always the selected browser. Firefox claims to have some weird background tab update system but it appears to be unnecessary.)

>+      ZoomManager.setZoomForBrowser(browser, 1);

>+      if (content.document.mozSyntheticDocument)
>+        return;
This (well, aBrowser.contentDocument) needs to be merged into the condition before the try/catch, as per _applySettingToPref (except that the latter correctly uses content.document).

>+    if (!(content.document.mozSyntheticDocument))
Nit: don't need the inner ()s any more.
Attachment #591902 - Flags: review-

Comment 4

6 years ago
Created attachment 602099 [details] [diff] [review]
Updates patch

Fixed the issues from the review comment.
BTW: I noticed Bug 722624 (Firefox), maybe we should fix this one as well while we're at it. Not sure what we could do, maybe save the current zoom level anyway when looking at an image or video? But that might not be what the user expects.
Attachment #591902 - Attachment is obsolete: true
Attachment #602099 - Flags: review?(neil)

Comment 5

6 years ago
Actually ignore what I said about Bug 722624, that one needs a different solution. Maybe storing the zoom level for pages (in contrast to images/video/...) somewhere in the backend.

Comment 6

6 years ago
Comment on attachment 602099 [details] [diff] [review]
Updates patch

Sorry for not spotting these last time.

>+    // Image documents should always start at 1, and are not affected by prefs.
>+    if (!aIsTabSwitch && aBrowser.contentDocument.mozSyntheticDocument) {
>+      ZoomManager.setZoomForBrowser(aBrowser, 1);
This needs to be this._ensureValid(1) as per bug 672146.

>-    if (!this.siteSpecific || window.gInPrintPreviewMode)
>+    if (!this.siteSpecific || window.gInPrintPreviewMode ||
>+        aBrowser.contentDocument.mozSyntheticDocument)
>       return;
>     var browser = aBrowser || (getBrowser() && getBrowser().selectedBrowser);
We had better move this up so that we can use browser instead of aBrowser.
(aBrowser could be null or undefined.)

r=me with those fixed.
Attachment #602099 - Flags: review?(neil) → review+

Comment 7

5 years ago
Created attachment 621341 [details] [diff] [review]
Patch for checkin
Attachment #602099 - Attachment is obsolete: true
Attachment #621341 - Flags: review+

Comment 8

5 years ago
Checked in: https://hg.mozilla.org/comm-central/rev/89874d58bafd
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → seamonkey2.12


5 years ago
Duplicate of this bug: 778427
You need to log in before you can comment on or make changes to this bug.