Last Comment Bug 658282 - Previously checked radio button does not get the invalid style
: Previously checked radio button does not get the invalid style
Status: VERIFIED FIXED
: testcase
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-19 08:28 PDT by Anthony Ricaud (:rik)
Modified: 2011-07-29 02:11 PDT (History)
5 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (459 bytes, text/html)
2011-05-19 08:28 PDT, Anthony Ricaud (:rik)
no flags Details
Part 1 - Improve css-invalid/input/ test suite (14.64 KB, patch)
2011-05-19 10:10 PDT, Mounir Lamouri (:mounir)
bzbarsky: review+
Details | Diff | Splinter Review
Part 2 - Actual fix. (3.08 KB, patch)
2011-05-19 10:12 PDT, Mounir Lamouri (:mounir)
bzbarsky: review+
Details | Diff | Splinter Review

Description Anthony Ricaud (:rik) 2011-05-19 08:28:27 PDT
Created attachment 533657 [details]
Testcase

See instructions in the testcase.
Comment 1 Anthony Ricaud (:rik) 2011-05-19 08:30:04 PDT
Also, I've only tested on my Mac. Might be a cross platform issue.
Comment 2 Mounir Lamouri (:mounir) 2011-05-19 08:50:39 PDT
Thanks for the testcase.
Comment 3 Boris Zbarsky [:bz] 2011-05-19 09:17:16 PDT
This worksforme in my build with the fix for bug 598833, fwiw.  The question is whether that's accidental...
Comment 4 Mounir Lamouri (:mounir) 2011-05-19 09:26:16 PDT
I'm improving a part of my test suite for <input type='radio'> and I think I catch this issue by doing that...
Comment 5 Mounir Lamouri (:mounir) 2011-05-19 10:10:44 PDT
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.
Comment 6 Mounir Lamouri (:mounir) 2011-05-19 10:12:20 PDT
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.
Comment 7 Mounir Lamouri (:mounir) 2011-05-19 10:13:30 PDT
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.
Comment 8 Boris Zbarsky [:bz] 2011-05-19 10:23:51 PDT
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)).
Comment 9 Boris Zbarsky [:bz] 2011-05-19 10:44:44 PDT
Comment on attachment 533689 [details] [diff] [review]
Part 1 - Improve css-invalid/input/ test suite

r=me
Comment 10 Mounir Lamouri (:mounir) 2011-05-20 07:01:52 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/b63ff702f39d
http://hg.mozilla.org/mozilla-central/rev/fca12da605c4

Thank you Anthony for reporting this.
Comment 11 Mounir Lamouri (:mounir) 2011-05-20 08:24:54 PDT
Follow-up for updating test suite in bug 658542.
Comment 12 Simona B [:simonab ] -PTO- back Sept 5th 2011-07-29 02:11:10 PDT
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.

Note You need to log in before you can comment on or make changes to this bug.