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

Status: RESOLVED FIXED

(SeaMonkey :: Tabbed Browser, defect)

Description

[Frank Wein [:mcsmurf]]

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

[Frank Wein [:mcsmurf]]

Attachment: Patch

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?

Comment 2

[Frank Wein [:mcsmurf]]

Comment on attachment 591902 [details] [diff] [review]
Patch

Not ready for review yet.

Comment 3

[[e-mail]]

Comment on attachment 591902 [details] [diff] [review]
Patch

>+    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);
aBrowser

>+      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.

Comment 4

[Frank Wein [:mcsmurf]]

Attachment: 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.

Comment 5

[Frank Wein [:mcsmurf]]

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

[[e-mail]]

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.

Comment 7

[Frank Wein [:mcsmurf]]

Attachment: Patch for checkin

Comment 8

[Frank Wein [:mcsmurf]]

Checked in: https://hg.mozilla.org/comm-central/rev/89874d58bafd