Closed
Bug 413433
Opened 17 years ago
Closed 17 years ago
session restore restores text zoom instead of full zoom
Categories
(Firefox :: Session Restore, defect)
Firefox
Session Restore
Tracking
()
RESOLVED
FIXED
Firefox 3 beta3
People
(Reporter: sragen, Assigned: zeniko)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
dietrich
:
review+
beltzner
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2 I accidentally scrolled up while closing a window. Upon opening Firefox again (restore session: on) the text was scaled up but images weren't. All sizes were treated as default and the normal sized images scaled down to very small images while the large text scaled down to normal sized text. Reproducible: Sometimes Steps to Reproduce: 1. Ctrl+UpScroll+W on last window with restore session on 2. Reopen Firefox 3. Ctrl+Scroll Actual Results: Images scaled all wanky Expected Results: Images should have been rendered at the size they should have been (IE: scaled up with the text) Default theme, nothing else of note.
Updated•17 years ago
|
Component: General → Session Restore
QA Contact: general → session.restore
Summary: image scaling not always correct → image scaling not always correct after session restore
sounds like it could be a combination of text zoom and full zoom...
Assignee | ||
Comment 2•17 years ago
|
||
Not sure how text and full zoom interact, but FWIW we only save and restore text zoom. I guess the right thing would be to use full zoom instead...
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Hardware: PC → All
Summary: image scaling not always correct after session restore → session restore restores text zoom instead of full zoom
Version: unspecified → Trunk
Definitely! The "Zoom" command now uses full zoom instead of text zoom.
Assignee | ||
Comment 4•17 years ago
|
||
Since fullZoom is the new textZoom, it's IMO not worth it teaching SessionStore to distinguish these two modes - we should just switch to restore fullZoom now so that when migrating from Fx 2 people can still Ctrl+0 to revert to a non-zoomed page.
Attachment #298961 -
Flags: review?(dietrich)
Comment 5•17 years ago
|
||
Actually, it may make sense to support both textZoom and fullZoom due to bug 401322.
Assignee | ||
Comment 6•17 years ago
|
||
Supporting both would be confusing for people who upgrade because they might get a page which is stuck at a text zoom level they can't change through the UI (or at least not without toggling a pref - should one get added in bug 401322). WRT bug 401322 I'd then just use whichever that pref dictates (for the same reason).
Depends on: 401322
Comment 7•17 years ago
|
||
Now that the current zoom level is remembered per site anyway (using the site- specific preferences), why is it necessary to save and restore the zoom level in SessionStore at all? (In reply to comment #6) > Supporting both would be confusing for people who upgrade because they might > get a page which is stuck at a text zoom level they can't change through the UI When the patch for bug 401322 lands, "View > Zoom > Reset" (or Ctrl + 0) will always reset both text and full zoom.
Assignee | ||
Comment 8•17 years ago
|
||
Comment on attachment 298961 [details] [diff] [review] s/textZoom/fullZoom/ (In reply to comment #7) > why is it necessary to save and restore the zoom level in SessionStore at all? Good question. AFAICT the only case where SessionStore would restore a different zoom setting than the site-specific prefs indicate is when restoring older sessions through a session managing extension. We could leave deciding what to do in that case to the extension, though... So ripping out those two zoom-related lines would be fine by me.
Attachment #298961 -
Flags: review?(dietrich)
Comment 9•17 years ago
|
||
(In reply to comment #8) > Good question. AFAICT the only case where SessionStore would restore a > different zoom setting than the site-specific prefs indicate is when restoring > older sessions through a session managing extension. Wouldn't in this case the site-specific pref override the one restored by the extension, at least when switching to a different tab and switching back, since the site-specific one is always applied when the URL changes? > So ripping out those two zoom-related lines would be fine by me. I think so too.
Assignee | ||
Comment 10•17 years ago
|
||
(In reply to comment #9) > Wouldn't in this case the site-specific pref override the one restored by the > extension, at least when switching to a different tab and switching back, At first thought, that's so counter-intuitive that I didn't even consider it. But as it currently stands, restoring zoom indeed doesn't make sense for SessionStore at all: Even if we used the ZoomManager to retrieve/restore zoom settings, restoring an older session would then just overwrite the site specific pref which IMO would be a rather unwanted side-effect. Let's rip zoom restoring out, then.
Attachment #298961 -
Attachment is obsolete: true
Attachment #299144 -
Flags: review?(dietrich)
Updated•17 years ago
|
Attachment #299144 -
Flags: review?(dietrich) → review+
Assignee | ||
Updated•17 years ago
|
Attachment #299144 -
Flags: approval1.9?
Comment 11•17 years ago
|
||
Comment on attachment 299144 [details] [diff] [review] rip out zoom restoring a=beltzner for 1.9
Attachment #299144 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Assignee: nobody → zeniko
Keywords: checkin-needed
Comment 12•17 years ago
|
||
Checking in browser/components/sessionstore/src/nsSessionStore.js; /cvsroot/mozilla/browser/components/sessionstore/src/nsSessionStore.js,v <-- nsSessionStore.js new revision: 1.93; previous revision: 1.92 done
Status: NEW → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M11
Updated•17 years ago
|
Flags: in-litmus?
Comment 13•15 years ago
|
||
in-litmus+ https://litmus.mozilla.org/show_test.cgi?id=7799
Flags: in-litmus? → in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•