[Webkit-unassigned] [Bug 75478] <summary> is not keyboard accessible

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=75478


MORITA Hajime <morrita at google.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #126029|review?                     |review-
               Flag|                            |




--- Comment #28 from MORITA Hajime <morrita at google.com>  2012-02-09 03:14:19 PST ---
(From update of attachment 126029)
View in context: https://bugs.webkit.org/attachment.cgi?id=126029&action=review

Arko, Thank you for updating the patch. It looks better!
I added some comments.

> Source/WebCore/html/HTMLSummaryElement.cpp:128
> +

Could you check BaseButtonInputType.cpp and HTMLInputElement.cpp to align the behavior? 
In my understanding, it 
- handles DOMActivate event instead of click event (I admit that the current implementation is wrong.)
- has different behaviors for each of enter key and space keys.
- uses Node::dispatchSimulatedClick() to convert key events to click, which eventually triggers DOMActivate event.

> LayoutTests/fast/html/details-keyboard-show-hide.html:24
> +

In general, layout tests should follow some specific patterns.
- You should split action (sending a key event) and check the result (if the open property is expected value) separately.
- For the value comparison, you can use shouldBe() family.
- Emits only minimal logs. Ideally the expectation contains a description line and set of PASS lines.

You can follow the style of well-written tests like ValidityState-valueMissing-002.html.
I admit (again!) that not all existing detail related tests does follow such style.
But I hope new tests to have more "usual" style.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.