236071 - Tabbing from a selection in a <legend> focuses wrong element

Status: VERIFIED FIXED

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

Description

[Mats Palmgren (inactive)]

Tabbing from a selection in a <legend> focuses wrong element.

STEPS TO REPRODUCE:
1. Load URL  (which is the testcase for bug 81481)
2. Select the text "Personal Information"
3. Press the TAB key

ACTUAL RESULTS:
The "Smallpox" checkbox receives focus.

EXPECTED RESULTS:
The text input after the text "Last Name:" should have received focus.

PLATFORMS & BUILDS TESTED:
Bug occurs in 2004-02-26-08 trunk on Linux
Bug occurs in Mozilla 1.6, 1.5, 1.4, 1.2.1 and 1.0.2 on Linux.

ADDITIONAL INFORMATION:
Bug does not occur in Opera 7.23 on Linux.

Comment 1

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

Swap the order of the fieldset child frames (only 2: Area and Legend).
This seems to work... (Warning: not very well tested yet)

Comment 2

[Mats Palmgren (inactive)]

mLegendFrame can be null I suppose...

Comment 3

[Mats Palmgren (inactive)]

Attachment: Patch rev. 2

Comment 4

[Boris Zbarsky [:bzbarsky]]

Fun...  ;)

Comment 5

[Mats Palmgren (inactive)]

Attachment: Patch rev. 3

Sure, and it got even funnier when stumbling into bug 70648 ;-)

This patch fixes the problem reported in this bug.
I have verified that the wallpaper for bug 70648 here also still works.
Btw, I still see the assertion Attanasi mentioned in bug 70648 comment 20:

###!!! ASSERTION: messed up delete code: 'flow == aDeletedFrame->GetParent()',
file nsBlockFrame.cpp, line 4729
Break: at file nsBlockFrame.cpp, line 4729

I ran the block regression tests with the expected results:
frame type mismatch: Area(fieldset)(3) vs. Legend(legend)(0)

I've checked them manually and ran some other tests too and
everything seems fine.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Mats, I'll try to take a look this coming weekend.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 142788 [details] [diff] [review]
Patch rev. 3

r=bzbarsky.  Add some comments in the frame constructor code explaining why we
put them in that order?

Comment 8

[Mats Palmgren (inactive)]

Attachment: Patch rev. 4

Added a comment, no other changes.

Comment 9

[Mats Palmgren (inactive)]

Attachment: Patch rev. 5

Updated to trunk with one addition since rev. 4.
In nsCSSFrameConstructor.cpp, GetAdjustedParentFrame() we need to return
the second child frame, if present.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 169868 [details] [diff] [review]
Patch rev. 5

r+sr=bzbarsky, with comments in the two nsCSSFrameConstructor hunks pointing to
each other ("change there if you change here" type things).

Comment 11

[Mats Palmgren (inactive)]

Attachment: Patch rev. 5 (checked in) - with added comments as requested

Comment 12

[Mats Palmgren (inactive)]

Checked in 2005-01-02 06:30 PDT.

-> FIXED

Comment 13

[Hiro]

Focus doesn't move to the middle box("Medical History" box) of the test case
when only the Tab key is pushed. 

STEPS TO REPRODUCE:
1.Open test case(URL)
2.Focus is moved to the "Address:" text field of Personal Information. 
3.Pushd Tab key.

ACTUAL RESULTS:
Focus doesn't move to "Smallpox" checkbox. 

EXPECTED RESULTS:
Focus moves to "Smallpox" checkbox. 

Mac OS X 10.3.7
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; ja-JP; rv:1.8a6) Gecko/20050103
Firefox/1.0+

Comment 14

[Mats Palmgren (inactive)]

(In reply to comment #13)
Works for me using Firefox 20050103 and local Mozilla debug build on Linux.
Works for me using Mozilla trunk 2005-01-03-07 on Windows XP.
Please file a separate bug report for MacOSX and CC me, if you can reproduce it
in an official mozilla.org build:
ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

Comment 15

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED; this works correctly for me using the testcase
https://bugzilla.mozilla.org/attachment.cgi?id=94558&action=view in comment 0 in
a Windows XP Seamonkey trunk 2005-01-17-07 build.

Comment 16

[Mats Palmgren (inactive)]

This caused bug 294717