632379 - Can't select some bookmarks

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[dauge]

User-Agent:       Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b11
Build Identifier: Mozilla/5.0 (Windows NT 5.1; rv:2.0b11) Gecko/20100101 Firefox/4.0b12

bookmarks view scroll down to bottom folders.  can't get to links in these foldes.  links that are in these folders display improperly.

Reproducible: Always

Steps to Reproduce:
1.view
2.bookmarks
3.scroll towards bottom of folders
can't get mouse to links in these folders

Comment 1

[dauge]

This is a bug for the latest nightly of beta 12.  I had backed up to beta 11 because of the problem.

In steps to reproduce and the description of problem ignore the "view" part of it.  This is incorrect.

Comment 2

[Marco Bonardo [:mak]]

which nightly exactly, could you please past here the user agent string of that nightly from about:support?
What happens exactly? do those entries appear wrongly or they just don't react to clicks?

Comment 3

[Alice0775 White]

Confirmed.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110208 Firefox/4.0b12pre ID:20110208030358

Regression window(cached m-c hourly):
Works;
http://hg.mozilla.org/mozilla-central/rev/16cc18d74a8c
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110205 Firefox/4.0b12pre ID:20110206175132
Fails:
http://hg.mozilla.org/mozilla-central/rev/4c62984f12d1
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110207 Firefox/4.0b12pre ID:20110207030345
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=16cc18d74a8c&tochange=4c62984f12d1

Comment 4

[Alice0775 White]

Screen capture
http://www.youtube.com/watch?v=h50598lWqF0

Comment 5

[Marco Bonardo [:mak]]

interesting, LTR vertical scrollboxes should have not been changed by that fix... looks like they were

Comment 6

[Alice0775 White]

Attachment: sample xul

Comment 8

[(no longer active)]

Assigning back to Simon, but I may get to look at it sooner that he does, in which case I'll reassign back to me.

Comment 10

[(no longer active)]

The fix to this bug should be tested to make sure that it also fixes bug 632938.

Comment 11

[WildcatRay]

(In reply to comment #10)
> The fix to this bug should be tested to make sure that it also fixes bug
> 632938.

Good point.

Comment 12

[Simon Montagu :smontagu]

Attachment: Patch

This was a consequence of not positioning the rect in LayoutScrollArea before calling Layout.

I'm having trouble writing a test for this. My idea was to open menupopups in a mochitest, with and without scrolling the parent menu, and compare the results of getBoundingClientRect for the submenu. The problem seems to be that getBoundingClientRect returns the place where the submenu should be, not the place where it actually is -- similarly to the way that the mouseover in the testcase responds to the place where the submenu should be and not where it actually is.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 511418 [details] [diff] [review]
Patch

r=dbaron

Comment 14

[Simon Montagu :smontagu]

Attachment: Patch with hg preamble

Comment 15

[Simon Montagu :smontagu]

Attachment: Test

I am checking this out on tryserver: 
http://tbpl.mozilla.org/?tree=MozillaTry&rev=96c67c9edce9 (without the fix)
http://tbpl.mozilla.org/?tree=MozillaTry&rev=357ac3ad571b (with the fix)

Comment 16

[(no longer active)]

I'm holding off in landing this until we get try server results.

Comment 17

[(no longer active)]

The test seems to be failing with the fix:

ERROR TEST-UNEXPECTED-FAIL | /tests/layout/generic/test/test_bug632379.xul | Popup should open in the same place when the menu is scrolled - got 88, expected 91

Comment 18

[Simon Montagu :smontagu]

All linux tryservers fail with "Popup should open in the same place when the menu is scrolled - got 88, expected 91"
All OSX tryservers timeout (presumably because |synthesizeKey("M", {altKey: true});| doesn't open the menu)
Windows tryservers pass except for Win XP Opt, which fails with "Popup should open in the same place when the menu is scrolled - got 77, expected 100"

Ehsan, do you have a better idea for a test?

Comment 20

[Simon Montagu :smontagu]

Attachment: Test

This version passes on linux and windows tryservers -- the difference is that I begin by opening the submenu on line 5, instead of line 3. This ensures that the toplevel menu scrolls down so that the opened menu item is exactly at the bottom of the scrollbox in both parts of the test. The test still fails without the patch, because it doesn't matter if the toplevel menu is scrolled both times that the submenu opens; as long as the amount of scrolling is different the bug still occurs.

I still need to find the right SynthesizeKey incantation to open the menu on Mac.

Comment 21

[(no longer active)]

(In reply to comment #20)
> I still need to find the right SynthesizeKey incantation to open the menu on
> Mac.

Can't you just open the menus with openPopup?  Getting accesskeys to work on Mac is tricky, if possible at all.

Comment 22

[Marco Bonardo [:mak]]

well, on Mac you are targeting Minimize Window...
According to this page (https://developer.mozilla.org/en/XUL_Accesskey_FAQ_and_Policies) "On Macintosh, accesskeys are available only in HTML not in XUL, and they are activated using CTRL+letter instead of ALT"
Maybe synthesizeMouseAtCenter?

Comment 23

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I think we should land the fix while you work on the test.

Comment 24

[Simon Montagu :smontagu]

Fix landed http://hg.mozilla.org/mozilla-central/rev/51702867d932

Comment 27

[Simon Montagu :smontagu]

Attachment: test

KISS: $("mainMenu").open works on all platforms http://tbpl.mozilla.org/?tree=MozillaTry&rev=7d9c9b4f6651

Comment 28

[Simon Montagu :smontagu]

Test landed http://hg.mozilla.org/mozilla-central/rev/26421a3b68f3