Closed
Bug 407601
Opened 17 years ago
Closed 17 years ago
Annoyance: always returns to the top of the bookmarks menu
Categories
(Firefox :: Bookmarks & History, defect, P4)
Firefox
Bookmarks & History
Tracking
()
VERIFIED
FIXED
Firefox 3 beta3
People
(Reporter: nicolastheadept, Assigned: enndeakin)
References
Details
(Keywords: regression, ue)
Attachments
(1 file, 2 obsolete files)
12.70 KB,
patch
|
bzbarsky
:
review+
bzbarsky
:
superreview+
mtschrep
:
approval1.9+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1 If the bookmarks menu is closed and then reopened (either by clicking a bookmark, or closing the menu), then it returns to the top of the menu. In previous versions of Firefox, it remained at its previous position. The new behaviour is extremely annoying when using a bookmarks menu that is longer than the screen, because it involves scrolling back through all the bookmarks that you just scrolled through. Please revert to the old behaviour. Reproducible: Always Steps to Reproduce: 1. Have lots of bookmarks (more than can be displayed at once) 2. Open the bookmarks menu 3. Scroll down and click on a bookmark 4. Open the bookmarks menu again Actual Results: It was back at the top of the bookmarks menu Expected Results: Remained at the same place in the menu.
Comment 1•17 years ago
|
||
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120823 Minefield/3.0b2pre
Status: UNCONFIRMED → NEW
Ever confirmed: true
Reporter | ||
Comment 2•17 years ago
|
||
Please could someone else confirm on non-Windows: Linux or OSX
Comment 3•17 years ago
|
||
Confirmed in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b2pre) Gecko/2007120804 Minefield/3.0b2pre.
Comment 4•17 years ago
|
||
Regression range for this is http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1190117820&maxdate=1190126399 With two possible culprits in it: bug 394301 and (less likely) bug 396412.
Blocks: 394301
Flags: blocking-firefox3?
Keywords: regression
OS: Windows Vista → All
Hardware: PC → All
Comment 5•17 years ago
|
||
I don't think this blocks, but we would gladly take a fix.
Flags: wanted-firefox3+
Flags: blocking-firefox3?
Flags: blocking-firefox3-
Priority: -- → P4
Target Milestone: --- → Firefox 3 M11
Assignee | ||
Comment 6•17 years ago
|
||
This is a possible patch and a better fix for bug 394301. It changes the positioning to use the original preferred size when calculating the position and size rather than the current size, as the current size can change if the popup is too close to the edge of the screen. This led to a size change and an underflow event being fired which caused the scrollbox to scroll to the top.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Assignee | ||
Comment 9•17 years ago
|
||
Attachment #292643 -
Attachment is obsolete: true
Attachment #293536 -
Flags: superreview?(bzbarsky)
Attachment #293536 -
Flags: review?(bzbarsky)
Comment 10•17 years ago
|
||
I'm not sure when I'll be able to get to this... might well not be until January if not later. Part of the problem is that this code is complex and I just don't remember it very well, so would have to relearn it. Some explanation of why the various parts are needed (if there is more to it than what is described in comment 6) would go a long way toward making this easier to review.
Assignee | ||
Comment 11•17 years ago
|
||
Although the patch looks big, it actually just changes the references to the size fields of mRect to a use different rect mPrefSize, so that positon and size computations are performed on the preferred size (returned by nsBox::GetPrefSize) rather than the current size in mRect. This is because the preferred size only changes when the content changes size, whereas mRect can change size (within SetPopupPosition) due to various things, like not enough screen space, the scroll arrows appearing on the menu, etc. This is because we want to computations to be based on the original preferred size, rather than the current size which keeps changing. This is only a wanted bug though.
Comment 12•17 years ago
|
||
Neil, could this fix the problem with drag&drop in bookmarks menu where the drop point is calculated without counting the popup movement due to not enough screen space? if so this is a wanted bug that can solve a blocker (i'm talking about Bug 405087), when comparing y on repositioned submenus they have still old y values.
Assignee | ||
Comment 13•17 years ago
|
||
Comment on attachment 293536 [details] [diff] [review] same as last patch, but modify a test so it checks the scroll position Found an issue with this patch anyway, and have a much simpler patch
Attachment #293536 -
Flags: superreview?(bzbarsky)
Attachment #293536 -
Flags: review?(bzbarsky)
Assignee | ||
Comment 14•17 years ago
|
||
Attachment #293536 -
Attachment is obsolete: true
Attachment #293676 -
Flags: superreview?(bzbarsky)
Attachment #293676 -
Flags: review?(bzbarsky)
Comment 15•17 years ago
|
||
Comment on attachment 293676 [details] [diff] [review] this patch is simpler and just initializes mRect to the preferred size before positioning/sizing the popup r+sr=bzbarsky
Attachment #293676 -
Flags: superreview?(bzbarsky)
Attachment #293676 -
Flags: superreview+
Attachment #293676 -
Flags: review?(bzbarsky)
Attachment #293676 -
Flags: review+
Assignee | ||
Updated•17 years ago
|
Attachment #293676 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #293676 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Updated•17 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Comment 16•17 years ago
|
||
verified with: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007122005 Minefield/3.0b3pre
Status: RESOLVED → VERIFIED
Assignee | ||
Updated•16 years ago
|
Flags: in-testsuite+
Comment 20•15 years ago
|
||
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h". In Thunderbird 3.0b, you do that as follows: Tools | Message Filters Make sure the correct account is selected. Click "New" Conditions: Body contains places-to-b-and-h Change the action to "Delete Message". Select "Manually Run" from the dropdown at the top. Click OK. Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter. Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in
before you can comment on or make changes to this bug.
Description
•