Last Comment Bug 721474 - Port |Bug 575830 - Image zoom (Page zoom) is reset when I switch tabs| to SeaMonkey
: Port |Bug 575830 - Image zoom (Page zoom) is reset when I switch tabs| to Sea...
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Tabbed Browser (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: seamonkey2.12
Assigned To: Frank Wein [:mcsmurf]
:
Mentors:
: 778427 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-26 11:51 PST by Frank Wein [:mcsmurf]
Modified: 2012-07-28 09:14 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch (1.95 KB, patch)
2012-01-26 13:06 PST, Frank Wein [:mcsmurf]
neil: review-
Details | Diff | Review
Updates patch (2.18 KB, patch)
2012-03-01 14:00 PST, Frank Wein [:mcsmurf]
neil: review+
Details | Diff | Review
Patch for checkin (2.27 KB, patch)
2012-05-05 13:49 PDT, Frank Wein [:mcsmurf]
bugzilla: review+
Details | Diff | Review

Description Frank Wein [:mcsmurf] 2012-01-26 11:51:31 PST
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] 2012-01-26 13:06:21 PST
Created attachment 591902 [details] [diff] [review]
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] 2012-01-26 13:08:24 PST
Comment on attachment 591902 [details] [diff] [review]
Patch

Not ready for review yet.
Comment 3 neil@parkwaycc.co.uk 2012-01-26 13:24:47 PST
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] 2012-03-01 14:00:35 PST
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.
Comment 5 Frank Wein [:mcsmurf] 2012-03-01 14:04:42 PST
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 neil@parkwaycc.co.uk 2012-03-01 15:53:30 PST
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] 2012-05-05 13:49:32 PDT
Created attachment 621341 [details] [diff] [review]
Patch for checkin
Comment 8 Frank Wein [:mcsmurf] 2012-05-16 01:59:06 PDT
Checked in: https://hg.mozilla.org/comm-central/rev/89874d58bafd
Comment 9 Philip Chee 2012-07-28 09:14:35 PDT
*** Bug 778427 has been marked as a duplicate of this bug. ***

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