Closed
Bug 551193
Opened 15 years ago
Closed 15 years ago
page position changes on backgrounds tabs when the foreground tab is scrolled
Categories
(Firefox for Android Graveyard :: Panning/Zooming, defect)
Tracking
(fennec1.1+)
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
fennec | 1.1+ | --- |
People
(Reporter: aakashd, Assigned: mbrubeck)
References
Details
Attachments
(1 file, 2 obsolete files)
2.41 KB,
patch
|
Details | Diff | Splinter Review |
Build Id:
Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2.2pre) Gecko/20100309 Namoroka/3.6.2pre Fennec/1.1a1
and
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.2pre) Gecko/20100309 Namoroka/3.6.2pre Fennec/1.1a2pre
and
Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a2pre) Gecko/20100309 Namoroka/3.7a2pre Fennec/1.1a2pre
Steps to Reproduce:
1. Go to www.google.com/news
2. Open a new tab and go to www.espn.com
3. Go back to the first tab with the loaded Google News page
4. Pan down to the middle of the page
5. Go to the second tab with the loaded ESPN page
Actual Results:
The page on the second tab, ESPN, will be panned past its top nav bar
Expected Results:
The page on the second tab should not be panned
Updated•15 years ago
|
tracking-fennec: ? → 1.1+
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → mbrubeck
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•15 years ago
|
||
It seems the urlbar position is shared across tabs. Changing it on one tab changes it on all tabs where the urlbar is visible. If you pan until it is half-visible, it will be half-visible on the other tabs. (Tabs that are scrolled all the way past the urlbar are not affected.) Investigating...
Assignee | ||
Comment 2•15 years ago
|
||
We were saving and restoring the #content-scrollbox offset per tab, but not the #page-scrollbox offset. (#page-scrollbox contains the urlbar and #content-scrollbox.)
Here's a patch that fixes the issue by also saving the #page-scrollbox offset. I'd appreciate some feedback.
* Should the old "scrollOffset" name be changed to "contentScrollOffset" (for symmetry with "pageScrollOffset")?
* There is one place where scrollOffset is set but pageScrollOffset is not, in ProgressController.prototype._documentStop. I think it's probably not necessary to change this, but I wasn't sure.
* Is this actually the best way to solve this problem, or is there something I didn't think of?
Attachment #434356 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 3•15 years ago
|
||
Attachment #434356 -
Attachment is obsolete: true
Attachment #434358 -
Flags: review?(mark.finkle)
Attachment #434356 -
Flags: review?(mark.finkle)
Assignee | ||
Comment 4•15 years ago
|
||
I've discovered a problem with this patch; for some reason the scroll position when Fennec first launches is off slightly. Looking into it.
Assignee | ||
Comment 5•15 years ago
|
||
Nevermind, it looks like the problem in comment 4 was already fixed by my revised patch; I was just testing an old build.
Assignee | ||
Comment 6•15 years ago
|
||
Err, nevermind again. The problem in comment 4 is still there, but unrelated to my patch. (The problem exists both with and without my patch.) Will file a separate bug if there's not one already.
Comment 7•15 years ago
|
||
Comment on attachment 434358 [details] [diff] [review]
don't access pageScrollOffset if it's not defined
Code looks OK. I want to test it a bit too.
Also, adding Ben to take a look too.
Attachment #434358 -
Flags: review?(webapps)
Attachment #434358 -
Flags: review?(mark.finkle)
Attachment #434358 -
Flags: review+
Comment 8•15 years ago
|
||
Comment on attachment 434358 [details] [diff] [review]
don't access pageScrollOffset if it's not defined
Ugh, someday we'll have to do a prettier job of storing scrollbox state for individual tabs.
Looks fine. Please move the XXX incorrect behavior above the first if statement so it looks more like it applies to both comments.
And please rename scrollOffset to contentScrollOffset.
Attachment #434358 -
Flags: review?(webapps) → review+
Assignee | ||
Comment 9•15 years ago
|
||
Attachment #434358 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed
Comment 10•15 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•15 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 11•15 years ago
|
||
verified FIXED on builds:
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.3pre) Gecko/20100329 Namoroka/3.6.3pre Fennec/1.1a2pre
and
Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.3a4pre) Gecko/20100329 Namoroka/3.7a4pre Fennec/1.1a2pre
litmus testcase https://litmus.mozilla.org/show_test.cgi?id=7587 regression tests this bug.
Status: RESOLVED → VERIFIED
Flags: in-litmus+
You need to log in
before you can comment on or make changes to this bug.
Description
•