[Webkit-unassigned] [Bug 71854] Access key should work on all elements

Thu Nov 17 04:22:38 PST 2011

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

Vineet Chaudhary (vineetc) <rgf748 at motorola.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #115012|0                           |1
        is obsolete|                            |
 Attachment #115558|                            |review?
               Flag|                            |

--- Comment #35 from Vineet Chaudhary (vineetc) <rgf748 at motorola.com>  2011-11-17 04:22:37 PST ---
Created an attachment (id=115558)
 --> (https://bugs.webkit.org/attachment.cgi?id=115558&action=review)
Updated Patch

(In reply to comment #33)

> > Source/WebCore/ChangeLog:9
> > +        says All HTML elements can have the accesskey content attribute set. Adding "accessKey"
> Nit: says All?
Corrected.

> > Source/WebCore/ChangeLog:10
> > +        attribute idl file as [Reflect].
> Please make 'Adding "accessKey" attribute idl file as [Reflect]' a full sentence.
Corrected.

> > Source/WebCore/ChangeLog:14
> > +        * bindings/objc/PublicDOMInterfaces.h: Moving properties form subclass to base class.
> s/Moving/Moved/
Corrected. 

> > LayoutTests/fast/forms/access-key-for-all-elements-expected.txt:1
> > +This test checks to see if accesskey attributes works all elements.
> Nit: works *on* all elements
Corrected.

> > LayoutTests/fast/forms/access-key-for-all-elements.html:1
> > +<html>
> No DOCTYPE?
Corrected.

> > LayoutTests/fast/forms/access-key-for-all-elements.html:10
> > +                     "body","br","button","canvas","caption","center","cite","code","col","colgroup","command","datalist","dd","del","details","dfn","dir","div","dl","dt",
> We don't normally indent like this. Just intend by 4 spaces please.
Corrected.

> > LayoutTests/fast/forms/access-key-for-all-elements.html:19
> > +    var parent = document.createElement(tagNames[i]);
> > +    document.body.appendChild(parent);
> 
> This isn't a parent, right? This IS the test element.
Corrected.
> > LayoutTests/fast/forms/access-key-for-all-elements.html:20
> > +    var testElement = document.querySelector(tagNames[i]);
> 
> Why do we need to query-select it?
Removed as using direct created element. 
> > LayoutTests/fast/forms/access-key-for-all-elements.html:25
> > +    accesskey = testElement.accessKey; 
> > +    shouldBe('accesskey',"'k'");
> 
> Why do we ned to copy it to accesskey here? You could have just written
> shouldBe('testElement.accessKey',"'k'");
Corrected.

Also modified test to make sure accessKey is actually working.
Checking onclick/onfocus events are fired for elements.

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