Mouse scrolling of long menus breaks if the last item is hidden via display:none

RESOLVED FIXED in mozilla31

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: mozilla, Assigned: alice0775)

Tracking

1.9.2 Branch
mozilla31
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Reporter

Description

6 years ago
User Agent: Mozilla/5.0 (X11; Linux i686; rv:17.0) Gecko/20100101 Firefox/17.0
Build ID: 20130107124423

Steps to reproduce:

1. Hide the last item of the bookmarks menu (eg: set menu_unsortedBookmarks to display: none in CSS).
2. Add enough bookmarks that scroll arrows appear on the menu.
3. Open bookmarks menu and twirl the mouse wheel.


Actual results:

1. Nothing.


Expected results:

1. The menu should scroll to show items which didn't fit on the screen.

Seems like this applies to all scrollboxes but I tested it with the bookmarks menu.  Adding non-blank items to the end of the menu makes scrolling work again, even invisible ones such as a zero-size spacer or a collapsed item.

Scrolling by hovering on the arrows works, this is scrollByPixel() rather than scrollByItem().  Should they both be the same?
Component: Untriaged → Bookmarks & History
Assignee

Comment 1

6 years ago
Posted file sample xul
you should use visibility:collapse instead of display:none.
Assignee

Updated

6 years ago
Component: Bookmarks & History → XUL
OS: Linux → All
Product: Firefox → Core
Version: 17 Branch → 1.9.2 Branch
Assignee

Comment 2

6 years ago
Regression window:
Good:
http://hg.mozilla.org/mozilla-central/rev/68cfe7fb9f31
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090413 Minefield/3.6a1pre ID:20090413045916
Bad:
http://hg.mozilla.org/mozilla-central/rev/68d9acc70491
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2a1pre) Gecko/20090414 Minefield/3.6a1pre ID:20090414100052
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=68cfe7fb9f31&tochange=68d9acc70491
Suspected: Bug 487946
Blocks: 487946
Assignee

Comment 3

6 years ago
This triggered by 
  10528984d5fd	Dão Gottwald — Bug 487982 - arrowscrollbox.scrollByIndex fails if there are gaps between the elements. r=enn
Blocks: 487982
No longer blocks: 487946
Component: XUL → XUL Widgets
Product: Core → Toolkit
Reporter

Comment 4

6 years ago
(In reply to Alice0775 White from comment #1)
> you should use visibility:collapse instead of display:none.

"visibility:collapse" is not a suitable alternative to "display: none" in this case.  Menuitems with "visibility: collapse" can still be tabbed to and can still be activated using the keyboard, causing much confusion since they are invisible.
Assignee

Comment 5

6 years ago
Posted patch possible fixSplinter Review
cannot scroll to 'display:none' element
Assignee

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment on attachment 734579 [details] [diff] [review]
possible fix

_canScrollToElement is called pretty frequently, so if we want to take this patch we need to make sure it doesn't perform significantly worse.
See Also: → 984156
Assignee

Updated

5 years ago
Attachment #734579 - Attachment is obsolete: true
Blocks: 984156
Comment on attachment 734579 [details] [diff] [review]
possible fix

I think this is probably ok performance-wise, as the code calling _canScrollToElement flushes layout anyway and layout shouldn't get dirty again between successive _canScrollToElement calls.

>       <method name="_canScrollToElement">
>         <parameter name="element"/>
>-        <body>
>-          return !element.hidden;
>-        </body>
>+        <body><![CDATA[
>+          return !element.hidden &&
>+                 window.getComputedStyle(element, null).getPropertyValue("display") != "none";
>+        ]]></body>
>       </method>

The !element.hidden check can be dropped.
Attachment #734579 - Attachment is obsolete: false
Attachment #734579 - Flags: review+
Assignee: nobody → alice0775
https://hg.mozilla.org/integration/fx-team/rev/c8b181197765
Summary: Mouse scrolling of long menus breaks if the last item is hidden → Mouse scrolling of long menus breaks if the last item is hidden via display:none
https://hg.mozilla.org/mozilla-central/rev/c8b181197765
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.