strange tab behaviour on lxr

RESOLVED FIXED

Status

()

--
major
RESOLVED FIXED
15 years ago
15 years ago

People

(Reporter: Biesinger, Assigned: aaronlev)

Tracking

(Blocks: 1 bug, {regression})

Trunk
x86
All
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

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

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

Comment 2

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

Updated

15 years ago
Blocks: 83552
(Assignee)

Comment 3

15 years ago
Happens on Windows too.
Severity: normal → major
OS: Linux → All
(Assignee)

Updated

15 years ago
Blocks: 127812
(Assignee)

Comment 5

15 years ago
Yes, it's my checkin.
Status: NEW → ASSIGNED
(Assignee)

Comment 6

15 years ago
Created attachment 148148 [details]
Test case
(Assignee)

Comment 7

15 years ago
Created attachment 148158 [details] [diff] [review]
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.
(Assignee)

Comment 8

15 years ago
Created attachment 148159 [details] [diff] [review]
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.
(Assignee)

Updated

15 years ago
Attachment #148158 - Attachment is obsolete: true
(Assignee)

Comment 9

15 years ago
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.
Attachment #148159 - Flags: review?(bryner)
(Assignee)

Updated

15 years ago
Blocks: 138069
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.
(Assignee)

Comment 11

15 years ago
Brian, I really don't think we want to allow tabbing inside of buttons -- that
makes me a little nervous.
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.
Attachment #148159 - Flags: review?(bryner) → review+
(Assignee)

Comment 13

15 years ago
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.
Attachment #148159 - Flags: superreview?(darin)

Comment 14

15 years ago
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)
Attachment #148159 - Flags: superreview?(darin) → superreview+
(Assignee)

Comment 15

15 years ago
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
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
*** Bug 244317 has been marked as a duplicate of this bug. ***
You need to log in before you can comment on or make changes to this bug.