Linux build 2003102905 To reproduce: 1. Open URL 2. Scroll to the right and choose some picture 3. RMB click and select "View Iamge" 4. Click "Back" button 5. Horizontal scroller position is NOT restored Expected result: Mozilla should restore horizontal scroller position like it does for vertical scroller
The problem here is the IBMBIDI code in nsGfxScrollFrameInner::AddHorizontalScrollbar that unconditionally resets the scroll position to 0 or 0x7FFFFFFF when the horizontal scrollbar is added. This is treated as a user scroll event and thus cancels session history restoration...
Assignee: history → misc
Component: History: Session → Layout: Misc Code
Comment on attachment 136135 [details] [diff] [review] Proposed patch roc, what do you think? Also, is this 1.6 material?
Assignee: misc → bz-vacation
Priority: -- → P2
Summary: Getting "Back" does not restore horizontal scroller position → [FIX]Getting "Back" does not restore horizontal scroller position
Target Milestone: --- → mozilla1.7alpha
This code looks okay, but I wonder if this is really the right fix. Do you think this would still work if we replaced the use of the "dir" attribute here with simple getting and setting of a member variable of nsGfxScrollFrameInner? I think it would work. But I wonder if a better fix for this BIDI issue would be to whack nsSliderFrame around here: http://lxr.mozilla.org/seamonkey/source/layout/xul/base/src/nsSliderFrame.cpp#247 Logically, in RTL you could think of the current pos as being the offset from the right hand side, not the left hand side. So we could modify the nsSliderFrame code for RTL scrollbar maxpos changes to keep maxpos - curpos constant. The only tricky part is how to stop *that* from being detected as a user scroll event. Where is that detection code BTW? Another thing that would be crazy but cool is if when curpos == maxpos and maxpos is increasing and curpos > 0, automatically update curpos to equal the new maxpos. Then when a page is loading and you scroll down to the bottom, as more of the page loads, we'll keep scrolling down automatically for you. What do you think? :-)
> Where is that detection code BTW? In nsScrollBoxFrame. Basically, any change in the scrolling view's position that is NOT triggered by a session history restore is treated as a user scroll event (makes sense). See the uses of mLastPos in that file. > Another thing that would be crazy but cool Yeah, we have existing bugs on the fact that all of our treatment of scrolling just kinda sucks like that (session history stuff, what happens when the window is resized, etc, etc). To be truthful, I just don't have the time or desire to reengineer all this code.
similar bug: Bug 99094
Okay fair enough. But since we're trying to reduce/eliminate the setting of attributes during reflow, could you change 'dir' here to be a member variable? Thanks. I don't think this is 1.6 material, BTW. It's not a high visibility bug and not a regression AFAIK.
OK. I'll try to move that into a member var.
Created attachment 137073 [details] [diff] [review] Patch with member
Attachment #136135 - Attachment is obsolete: true
Summary: [FIX]Getting "Back" does not restore horizontal scroller position → [FIXr]Getting "Back" does not restore horizontal scroller position
Checked in for 1.7a.
Status: NEW → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.