243028 - strange tab behaviour on lxr

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

gtk1 nightly, 2004050707
I'm pretty sure this is a regression

On LXR, hitting tab several times to reach the "file name" input field does not
work. instead, when reaching "file name search" and hitting tab, nothing seems
to be selected, and hitting tab again selects the link "file name search" again.

hm... actually this seems not limited to this box, "text search" has the same
issue, as does identifier search.

expected results: Hitting tab while "file name search" (the link) has focus,
should put the cursur into the textbox next to this link, and hitting tab again
should select the link "identifier search".

Comment 1

[[e-mail]]

I think it's the embedded <br> tags that are confusing Mozilla.

Comment 2

[Andrew Schultz]

regression between linux trunk 2004042807 and 2004050105 (comet's disk was full
between those builds)

Comment 3

[Aaron Leventhal]

Happens on Windows too.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

bonsai url for that: 
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=SeaMonkeyAll&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2004-04-28+07%3A00+&maxdate=2004-05-01+05%3A00&cvsroot=%2Fcvsroot

hm.. maybe aaronl's "Bug 138069. Make -moz-user-focus work for any html element.
r+sr=bryner"?

Comment 5

[Aaron Leventhal]

Yes, it's my checkin.

Comment 6

[Aaron Leventhal]

Attachment: Test case

Comment 7

[Aaron Leventhal]

Attachment: Only focus on the top element with -moz-user-focus, not each child element

The problem is that -moz-any-link is applied to all the children of a link, so
it looks like they're all focusable. Only the top link element should be
focused.

Comment 8

[Aaron Leventhal]

Attachment: Only focus on the top element with -moz-user-focus, not each child element

The problem is that -moz-any-link is applied to all the children of a link, so
it looks like they're all focusable. Only the top link element of the linked
subtree should be focused.

Comment 9

[Aaron Leventhal]

Comment on attachment 148159 [details] [diff] [review]
Only focus on the top element with -moz-user-focus, not each child element

Seeking r=bryner

This actually fixes something else I noticed -- backward tabbing through
something with -moz-user-focus and a :before element would stop twice instead
of just once.

Comment 10

[Brian Ryner (not reading)]

Comment on attachment 148159 [details] [diff] [review]
Only focus on the top element with -moz-user-focus, not each child element

Hm, I'm trying to think whether the assumptions you've made here are really
valid.	<button> elements can contain arbitrary content; with this patch you
could no longer tab to that content.  However, IE doesn't let you tab to the
link either (nor does it make it a link, really).  I'll test some other
browsers.

Comment 11

[Aaron Leventhal]

Brian, I really don't think we want to allow tabbing inside of buttons -- that
makes me a little nervous.

Comment 12

[Brian Ryner (not reading)]

Comment on attachment 148159 [details] [diff] [review]
Only focus on the top element with -moz-user-focus, not each child element

One comment -- Please make this just a |static| helper function instead of a
member.  It will make it a bit more performant.

Comment 13

[Aaron Leventhal]

Comment on attachment 148159 [details] [diff] [review]
Only focus on the top element with -moz-user-focus, not each child element

I'll make that static helper.

Comment 14

[Darin Fisher]

Comment on attachment 148159 [details] [diff] [review]
Only focus on the top element with -moz-user-focus, not each child element

>Index: nsEventStateManager.cpp

>+  while ((ancestorFrame = ancestorFrame->GetParent()) != NULL) {

nit: this code uses |nsnull| instead of |NULL|... maybe stick with
that convention?


>+    const nsStyleUserInterface* ui = ancestorFrame->GetStyleUserInterface();

nit: s/nsStyleUserInterface* ui/nsStyleUserInterface *ui/

just to be consistent with existing style and the rest of your patch.


sr=darin (but really more of a rubber stamp since i don't know this code)

Comment 15

[Aaron Leventhal]

Okay, I made your changes Darin. Thanks.

Checking in nsEventStateManager.cpp;
/cvsroot/mozilla/content/events/src/nsEventStateManager.cpp,v  <-- 
nsEventStateManager.cpp
new revision: 1.502; previous revision: 1.501
done
Checking in nsEventStateManager.h;
/cvsroot/mozilla/content/events/src/nsEventStateManager.h,v  <-- 
nsEventStateManager.h
new revision: 1.112; previous revision: 1.111
done

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

*** Bug 244317 has been marked as a duplicate of this bug. ***