Bug 117367 – IndieUI: Add basic IndieUI infrastructure

• Component: Accessibility
• Status: RESOLVED FIXED

[chris fleizach]

Add in the basic infrastructure for this feature
   The right defines
    UIRequestEvent (the base Event class)
    A layout test or two

[chris fleizach]

Created attachment 204266 [details]
patch

[WebKit Commit Bot]

Attachment 204266 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/indieui/create-uirequestevent-expected.txt', u'LayoutTests/indieui/create-uirequestevent.html', u'LayoutTests/indieui/uiactions-expected.txt', u'LayoutTests/indieui/uiactions.html', u'LayoutTests/platform/efl/TestExpectations', u'LayoutTests/platform/gtk/TestExpectations', u'LayoutTests/platform/mac/TestExpectations', u'LayoutTests/platform/qt/TestExpectations', u'Source/WebCore/CMakeLists.txt', u'Source/WebCore/ChangeLog', u'Source/WebCore/Configurations/FeatureDefines.xcconfig', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/GNUmakefile.am', u'Source/WebCore/Modules/indieui/UIRequestEvent.cpp', u'Source/WebCore/Modules/indieui/UIRequestEvent.h', u'Source/WebCore/Modules/indieui/UIRequestEvent.idl', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/dom/Element.cpp', u'Source/WebCore/dom/Element.h', u'Source/WebCore/dom/Element.idl', u'Source/WebCore/dom/ElementRareData.h', u'Source/WebCore/dom/EventNames.in', u'Source/WebCore/dom/Node.cpp', u'Source/WebCore/dom/Node.h', u'Source/WebCore/html/HTMLAttributeNames.in']" exit_code: 1
Source/WebCore/Modules/indieui/UIRequestEvent.h:46:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/Modules/indieui/UIRequestEvent.h:56:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/Modules/indieui/UIRequestEvent.cpp:53:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebCore/Modules/indieui/UIRequestEvent.cpp:65:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 4 in 26 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Created attachment 204269 [details]
patch

[chris fleizach]

Adding Tim to help with review

[Tim Horton]

Comment on attachment 204269 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204269&action=review

> Source/WebCore/ChangeLog:8
> +        Add basic support by adding the UIRequestEvent, uiactions attribute and 

Should there be an “and” here? And what are you adding the uiactions attribute to? And maybe a period?

> Source/WebCore/ChangeLog:10
> +

Some basic explanation of what IndieUI is might be nice here. Also, was there a webkit-dev email? I can’t find one.

> Source/WebCore/Configurations/FeatureDefines.xcconfig:100
> +ENABLE_INDIE_UI = ;

Should this be enabled, since we’re trying to keep all features on in ToT and allow ports to disable them on branches? Or is it not far enough along for that yet?

> Source/WebCore/Modules/indieui/UIRequestEvent.cpp:39
> +    bubbles = true;
> +    cancelable = true;

Why aren’t these in the initializer list as well?

> Source/WebCore/dom/Element.h:621
> +    void setuiactions(const AtomicString&);
> +    const AtomicString& uiactions() const;

Is there a reason these have bizarre and unusual capitalization? Should you use ImplementedAs so they don’t have to? (or, actually, it sort of looks like it might “just work” if you use the good capitalization here and the “bad” one in the IDL? I see other instances of that.) Why do they have the bizarre capitalization in the IDL too?

> Source/WebCore/dom/ElementRareData.h:75
> +    void setuiactions(const AtomicString&);
> +    AtomicString& uiactions() const;

Continuing with the capitalization. Maybe there’s something important I’m missing.

[chris fleizach]

(In reply to comment #5)
> (From update of attachment 204269 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204269&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Add basic support by adding the UIRequestEvent, uiactions attribute and 
> 
> Should there be an “and” here? And what are you adding the uiactions attribute to? And maybe a period?

Sure

> 
> > Source/WebCore/ChangeLog:10
> > +
> 
> Some basic explanation of what IndieUI is might be nice here. Also, was there a webkit-dev email? I can’t find one.

Will do. 
Here's the announcement
https://lists.webkit.org/pipermail/webkit-dev/2013-March/024044.html

> 
> > Source/WebCore/Configurations/FeatureDefines.xcconfig:100
> > +ENABLE_INDIE_UI = ;
> 
> Should this be enabled, since we’re trying to keep all features on in ToT and allow ports to disable them on branches? Or is it not far enough along for that yet?

I was going to wait until I had a few basic events implemented (value change and dismiss request) but I'm also OK enabling now as well. That will improve test coverage

> 
> > Source/WebCore/Modules/indieui/UIRequestEvent.cpp:39
> > +    bubbles = true;
> > +    cancelable = true;
> 
> Why aren’t these in the initializer list as well?

I look into it. Those are inherited so I might have had compile trouble

> 
> > Source/WebCore/dom/Element.h:621
> > +    void setuiactions(const AtomicString&);
> > +    const AtomicString& uiactions() const;
> 
> Is there a reason these have bizarre and unusual capitalization? Should you use ImplementedAs so they don’t have to? (or, actually, it sort of looks like it might “just work” if you use the good capitalization here and the “bad” one in the IDL? I see other instances of that.) Why do they have the bizarre capitalization in the IDL too?

The attribute is not supposed to have any capitalization (element.uiactions = "asdf") - I can look into using ImplementedAs to have better capitalization internally.

Would you expect to set
setUIActions()
UIAction()

> 
> > Source/WebCore/dom/ElementRareData.h:75
> > +    void setuiactions(const AtomicString&);
> > +    AtomicString& uiactions() const;
> 
> Continuing with the capitalization. Maybe there’s something important I’m missing.

[Tim Horton]

(In reply to comment #6)
> > > Source/WebCore/ChangeLog:10
> > > +
> > 
> > Some basic explanation of what IndieUI is might be nice here. Also, was there a webkit-dev email? I can’t find one.
> 
> Will do. 
> Here's the announcement
> https://lists.webkit.org/pipermail/webkit-dev/2013-March/024044.html

Aha. Excellent.

> > > Source/WebCore/dom/Element.h:621
> > > +    void setuiactions(const AtomicString&);
> > > +    const AtomicString& uiactions() const;
> > 
> > Is there a reason these have bizarre and unusual capitalization? Should you use ImplementedAs so they don’t have to? (or, actually, it sort of looks like it might “just work” if you use the good capitalization here and the “bad” one in the IDL? I see other instances of that.) Why do they have the bizarre capitalization in the IDL too?
> 
> The attribute is not supposed to have any capitalization (element.uiactions = "asdf") - I can look into using ImplementedAs to have better capitalization internally.
> 
> Would you expect to set
> setUIActions()
> UIAction()

Yep.

I’m not sure implementedas is the way to go, though. Other properties don’t seem to do that (except for prefixing, which is what it’s really for). Maybe this stuff is case insensitive? Bug someone who knows about bindings :)

[chris fleizach]

Created attachment 204959 [details]
patch to address feedback

[Build Bot]

Comment on attachment 204959 [details]
patch to address feedback

Attachment 204959 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/923411

New failing tests:
fast/js/global-constructors-attributes.html
fast/js/dom-static-property-for-in-iteration.html

[Build Bot]

Created attachment 204962 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[Ryosuke Niwa]

Comment on attachment 204959 [details]
patch to address feedback

View in context: https://bugs.webkit.org/attachment.cgi?id=204959&action=review

> Source/WebCore/ChangeLog:10
> +        IndieUI is a new W3C spec that aims to abstract the connection between input and action, so that
> +        a user (possibly using assistive technologies like a screen reader) does not need a specific kind
> +        of device (like a mouse) to interact with an object.

Where this feature defined? Could you add a URL to the specification?
Also, we should probably announce this feature on webkit-dev.

> Source/WebCore/Modules/indieui/UIRequestEvent.cpp:40
> +    // Member variables of the super class can't be initialized in the initializer list, hence they're set here.
> +    bubbles = true;
> +    cancelable = true;

Can't we add a new constructor in UIEvent instead?

[Ryosuke Niwa]

Comment on attachment 204959 [details]
patch to address feedback

View in context: https://bugs.webkit.org/attachment.cgi?id=204959&action=review

> Source/WebCore/ChangeLog:10
> +        IndieUI is a new W3C spec that aims to abstract the connection between input and action, so that
> +        a user (possibly using assistive technologies like a screen reader) does not need a specific kind
> +        of device (like a mouse) to interact with an object.

Where this feature defined? Could you add a URL to the specification?
Also, we should probably announce this feature on webkit-dev.

> Source/WebCore/Modules/indieui/UIRequestEvent.cpp:40
> +    // Member variables of the super class can't be initialized in the initializer list, hence they're set here.
> +    bubbles = true;
> +    cancelable = true;

Can't we add a new constructor in UIEvent instead?

[Tim Horton]

(In reply to comment #12)
> (From update of attachment 204959 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=204959&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        IndieUI is a new W3C spec that aims to abstract the connection between input and action, so that
> > +        a user (possibly using assistive technologies like a screen reader) does not need a specific kind
> > +        of device (like a mouse) to interact with an object.
> 
> Where this feature defined? Could you add a URL to the specification?
> Also, we should probably announce this feature on webkit-dev.

They already did, see above.

[Build Bot]

Comment on attachment 204959 [details]
patch to address feedback

Attachment 204959 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/924376

New failing tests:
indieui/uiactions.html
indieui/create-uirequestevent.html

[Build Bot]

Created attachment 204974 [details]
Archive of layout-test-results from APPLE-EWS-3 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-3  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[Build Bot]

Comment on attachment 204959 [details]
patch to address feedback

Attachment 204959 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/869337

New failing tests:
indieui/uiactions.html
indieui/create-uirequestevent.html

[Build Bot]

Created attachment 204978 [details]
Archive of layout-test-results from APPLE-EWS-4 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-4  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[chris fleizach]

Created attachment 205007 [details]
patch

[Build Bot]

Comment on attachment 205007 [details]
patch

Attachment 205007 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/869528

New failing tests:
fast/js/dom-static-property-for-in-iteration.html

[Build Bot]

Created attachment 205042 [details]
Archive of layout-test-results from webkit-ews-08 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-08  Port: mac-mountainlion  Platform: Mac OS X 10.8.3

[chris fleizach]

Created attachment 205044 [details]
patch

[chris fleizach]

(In reply to comment #21)
> Created an attachment (id=205044) [details]
> patch

Latest patch updates ChangeLog to include W3C URL and modifies EventInit so that you can pass in bubbles and cancelable for easier subclass initialization

[Build Bot]

Comment on attachment 205044 [details]
patch

Attachment 205044 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/886129

New failing tests:
fast/js/dom-static-property-for-in-iteration.html

[Build Bot]

Created attachment 205101 [details]
Archive of layout-test-results from APPLE-EWS-5 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-5  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[Build Bot]

Comment on attachment 205044 [details]
patch

Attachment 205044 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/936204

New failing tests:
fast/js/dom-static-property-for-in-iteration.html

[Build Bot]

Created attachment 205103 [details]
Archive of layout-test-results from APPLE-EWS-1 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-1  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[Ryosuke Niwa]

Comment on attachment 205044 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=205044&action=review

> LayoutTests/fast/js/dom-static-property-for-in-iteration-expected.txt:61
> +PASS a["uiactions"] is 

Since you're not enabling this feature on Qt and Windows, you need to svn cp this file into platform/qt and platform/win.

[Build Bot]

Comment on attachment 205044 [details]
patch

Attachment 205044 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/876448

New failing tests:
fast/js/dom-static-property-for-in-iteration.html

[Build Bot]

Created attachment 205109 [details]
Archive of layout-test-results from APPLE-EWS-3 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: APPLE-EWS-3  Port: win-future  Platform: CYGWIN_NT-6.1-WOW64-1.7.20-0.266-5-3-i686-32bit

[chris fleizach]

http://trac.webkit.org/changeset/151827