Add in the basic infrastructure for this feature The right defines UIRequestEvent (the base Event class) A layout test or two
Created attachment 204266 [details] patch
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.
Created attachment 204269 [details] patch
Adding Tim to help with review
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.
(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.
(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 :)
Created attachment 204959 [details] patch to address feedback
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
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
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?
(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.
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
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
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
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
Created attachment 205007 [details] patch
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
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
Created attachment 205044 [details] patch
(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
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
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
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
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
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.
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
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
http://trac.webkit.org/changeset/151827