Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Previously checked radio button does not get the invalid style

VERIFIED FIXED in mozilla6

Status

()

Core
Layout: Form Controls
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: rik, Assigned: mounir)

Tracking

({testcase})

Trunk
mozilla6
testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
Created attachment 533657 [details]
Testcase

See instructions in the testcase.
(Reporter)

Comment 1

6 years ago
Also, I've only tested on my Mac. Might be a cross platform issue.
(Assignee)

Comment 2

6 years ago
Thanks for the testcase.
Keywords: testcase
OS: Mac OS X → All
Hardware: x86 → All

Comment 3

6 years ago
This worksforme in my build with the fix for bug 598833, fwiw.  The question is whether that's accidental...
(Assignee)

Comment 4

6 years ago
I'm improving a part of my test suite for <input type='radio'> and I think I catch this issue by doing that...
(Assignee)

Comment 5

6 years ago
Created attachment 533689 [details] [diff] [review]
Part 1 - Improve css-invalid/input/ test suite

I need tests that check the actual rendering of radio buttons. I will probably have to open some follow-ups for all similar tests.
Assignee: nobody → mounir.lamouri
Status: NEW → ASSIGNED
Attachment #533689 - Flags: review?(bzbarsky)
(Assignee)

Comment 6

6 years ago
Created attachment 533690 [details] [diff] [review]
Part 2 - Actual fix.

Actually, the patch is quite trivial: UpdateValueMissingValidityStateForRadio doesn't notify about |this| state change because callers have to worry about that... But one was not doing that.
Attachment #533690 - Flags: review?(bzbarsky)
(Assignee)

Comment 7

6 years ago
BTW, there were a path for checkboxes and another for radio because the radio path was using a radio group visitor but now, UpdateAllValidityStates is going to call the correct method.
(Assignee)

Updated

6 years ago
Whiteboard: [needs review]

Comment 8

6 years ago
Ah, indeed.  The patch in bug 598833 had added an UpdateState() call there.  I guess I can remove it with this change (UpdateAllValidityStates already does UpdateState(aNotify)).

Updated

6 years ago
Attachment #533690 - Flags: review?(bzbarsky) → review+

Comment 9

6 years ago
Comment on attachment 533689 [details] [diff] [review]
Part 1 - Improve css-invalid/input/ test suite

r=me
Attachment #533689 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

6 years ago
Flags: in-testsuite+
Whiteboard: [needs review] → [fixed in cedar]
(Assignee)

Comment 10

6 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/b63ff702f39d
http://hg.mozilla.org/mozilla-central/rev/fca12da605c4

Thank you Anthony for reporting this.
Whiteboard: [fixed in cedar]
Target Milestone: --- → mozilla6
(Assignee)

Updated

6 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 11

6 years ago
Follow-up for updating test suite in bug 658542.
Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue on Windows XP, Windows 7, Mac OS X 10.6 and Ubuntu using the attached test case in Comment 0.

Setting resolution to VERIFIED FIXED.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.