Support the UIValueChangeRequestEvent https://dvcs.w3.org/hg/IndieUI/raw-file/default/src/indie-ui-events.html#dictionary-uivaluechangerequesteventinit-members
Created attachment 205303 [details] patch
Attachment 205303 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/global-constructors-attributes-expected.txt', u'LayoutTests/indieui/uivaluechangerequest-expected.txt', u'LayoutTests/indieui/uivaluechangerequest.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.cpp', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.idl', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/accessibility/AccessibilityNodeObject.cpp', u'Source/WebCore/accessibility/AccessibilityNodeObject.h', u'Source/WebCore/dom/EventNames.h', u'Source/WebCore/dom/EventNames.in', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/testing/Internals.h', u'Source/WebCore/testing/Internals.idl']" exit_code: 1 Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.cpp:49: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.cpp:61: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:44: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:45: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:46: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:47: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:48: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:49: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:50: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:51: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:52: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:57: When wrapping a line, only indent 4 spaces. [whitespace/indent] [3] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:69: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:70: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] Total errors found: 14 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 205303 [details] patch Attachment 205303 [details] did not pass qt-ews (qt): Output: http://webkit-queues.appspot.com/results/878076
Comment on attachment 205303 [details] patch Attachment 205303 [details] did not pass qt-wk2-ews (qt-wk2): Output: http://webkit-queues.appspot.com/results/856396
Created attachment 205307 [details] patch I think I have to ignore the style complaint about the all caps constants, since those are the names specified in the spec
Attachment 205307 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/global-constructors-attributes-expected.txt', u'LayoutTests/indieui/uivaluechangerequest-expected.txt', u'LayoutTests/indieui/uivaluechangerequest.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.cpp', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.idl', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/accessibility/AccessibilityNodeObject.cpp', u'Source/WebCore/accessibility/AccessibilityNodeObject.h', u'Source/WebCore/dom/EventNames.h', u'Source/WebCore/dom/EventNames.in', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/testing/Internals.h', u'Source/WebCore/testing/Internals.idl']" exit_code: 1 Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:44: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:45: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:46: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:47: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:48: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:49: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:50: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:51: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:52: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 9 in 18 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 205307 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=205307&action=review > Source/WebCore/ChangeLog:10 > + (Example, allow a screen reader user to change the value of a custom slider). It seems like this line can fit on the previous line. > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1019 > + DEFINE_STATIC_LOCAL(const AtomicString, noAction, ()); Why don't we use nullAtom or emptyAtom instead? > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1041 > + uiActionValue.split(' ', uiActions); The spec says it's a list of "a set of whitespace-separated tokens". I'm actually surprised that uiactions DOM property isn't DOMTokenList. Perhaps we should give that feedback to the standards body. Regardless, we should probably use DOMTokenList's algorithm to split the tokens here. We also need a test that uses non-space whitespaces. > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1061 > + UIValueChangeRequestEventInit eventInit; > + eventInit.changeType = increase ? UIValueChangeRequestEvent::INCREMENT : UIValueChangeRequestEvent::DECREMENT; > + eventInit.receiver = receiver; Can we add a constructor to do this? > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1065 > + RefPtr<UIValueChangeRequestEvent> event = UIValueChangeRequestEvent::create(eventNames().valuechangerequestEvent, eventInit); > + > + node()->dispatchUIRequestEvent(event); You can do both of these things in single line. > Source/WebCore/testing/Internals.cpp:2065 > + UIValueChangeRequestEventInit eventInit; > + eventInit.changeType = changeType; > + eventInit.receiver = receiver; Ditto. > Source/WebCore/testing/Internals.cpp:2069 > + RefPtr<UIValueChangeRequestEvent> event = UIValueChangeRequestEvent::create(eventNames().valuechangerequestEvent, eventInit); > + > + target->dispatchUIRequestEvent(event); Ditto.
Comment on attachment 205307 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=205307&action=review >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1041 >> + uiActionValue.split(' ', uiActions); > > The spec says it's a list of "a set of whitespace-separated tokens". I'm actually surprised that uiactions DOM property isn't DOMTokenList. > Perhaps we should give that feedback to the standards body. Regardless, we should probably use DOMTokenList's algorithm to split the tokens here. > We also need a test that uses non-space whitespaces. I'll inform the committee about this. I'll file another bug about testing uiactions with non space whitespaces >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1061 >> + eventInit.receiver = receiver; > > Can we add a constructor to do this? Yes. Will add some new eventInit constructors >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1065 >> + node()->dispatchUIRequestEvent(event); > > You can do both of these things in single line. If i do that I think the event will be leaked. I'm relying on the static allocation to dealloc the event
(In reply to comment #8) > (From update of attachment 205307 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=205307&action=review > > >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1041 > >> + uiActionValue.split(' ', uiActions); > > > > The spec says it's a list of "a set of whitespace-separated tokens". I'm actually surprised that uiactions DOM property isn't DOMTokenList. > > Perhaps we should give that feedback to the standards body. Regardless, we should probably use DOMTokenList's algorithm to split the tokens here. > > We also need a test that uses non-space whitespaces. > > I'll inform the committee about this. I'll file another bug about testing uiactions with non space whitespaces https://bugs.webkit.org/show_bug.cgi?id=118270 > > >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1061 > >> + eventInit.receiver = receiver; > > > > Can we add a constructor to do this? > > Yes. Will add some new eventInit constructors > > >> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1065 > >> + node()->dispatchUIRequestEvent(event); > > > > You can do both of these things in single line. > > If i do that I think the event will be leaked. I'm relying on the static allocation to dealloc the event
INDIEUI-ACTION-60: uiactions attr value should be a DOMTokenList https://www.w3.org/WAI/IndieUI/track/actions/60
Created attachment 206341 [details] patch This should address Ryosuke's comments
Attachment 206341 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast/js/global-constructors-attributes-expected.txt', u'LayoutTests/indieui/uivaluechangerequest-expected.txt', u'LayoutTests/indieui/uivaluechangerequest.html', u'Source/WebCore/ChangeLog', u'Source/WebCore/DerivedSources.make', u'Source/WebCore/Modules/indieui/UIRequestEvent.cpp', u'Source/WebCore/Modules/indieui/UIRequestEvent.h', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.cpp', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h', u'Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.idl', u'Source/WebCore/WebCore.exp.in', u'Source/WebCore/WebCore.xcodeproj/project.pbxproj', u'Source/WebCore/accessibility/AccessibilityNodeObject.cpp', u'Source/WebCore/accessibility/AccessibilityNodeObject.h', u'Source/WebCore/dom/EventNames.h', u'Source/WebCore/dom/EventNames.in', u'Source/WebCore/testing/Internals.cpp', u'Source/WebCore/testing/Internals.h', u'Source/WebCore/testing/Internals.idl']" exit_code: 1 Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:45: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:46: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:47: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:48: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:49: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:50: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:51: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:52: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Source/WebCore/Modules/indieui/UIValueChangeRequestEvent.h:53: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4] Total errors found: 9 in 20 files If any of these errors are false positives, please file a bug against check-webkit-style.
FYI, the uiactions IDL attribute is now a DOMTokenList, not a DOMString. The uiactions content attribute is still a string, of course. https://dvcs.w3.org/hg/IndieUI/rev/3ab9f0b62e4d
…which means you'll need to update this test at the very least: LayoutTests/indieui/uiactions.html
(In reply to comment #14) > …which means you'll need to update this test at the very least: > LayoutTests/indieui/uiactions.html That change will need to come in a different patch, and which point this test will get updated yes
<rdar://problem/14407328>
(In reply to comment #7) > (From update of attachment 205307 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=205307&action=review > > > Source/WebCore/ChangeLog:10 > > + (Example, allow a screen reader user to change the value of a custom slider). > > It seems like this line can fit on the previous line. > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1019 > > + DEFINE_STATIC_LOCAL(const AtomicString, noAction, ()); > > Why don't we use nullAtom or emptyAtom instead? > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1041 > > + uiActionValue.split(' ', uiActions); > > The spec says it's a list of "a set of whitespace-separated tokens". I'm actually surprised that uiactions DOM property isn't DOMTokenList. > Perhaps we should give that feedback to the standards body. Regardless, we should probably use DOMTokenList's algorithm to split the tokens here. > We also need a test that uses non-space whitespaces. > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1061 > > + UIValueChangeRequestEventInit eventInit; > > + eventInit.changeType = increase ? UIValueChangeRequestEvent::INCREMENT : UIValueChangeRequestEvent::DECREMENT; > > + eventInit.receiver = receiver; > > Can we add a constructor to do this? > > > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:1065 > > + RefPtr<UIValueChangeRequestEvent> event = UIValueChangeRequestEvent::create(eventNames().valuechangerequestEvent, eventInit); > > + > > + node()->dispatchUIRequestEvent(event); > > You can do both of these things in single line. > > > Source/WebCore/testing/Internals.cpp:2065 > > + UIValueChangeRequestEventInit eventInit; > > + eventInit.changeType = changeType; > > + eventInit.receiver = receiver; > > Ditto. > > > Source/WebCore/testing/Internals.cpp:2069 > > + RefPtr<UIValueChangeRequestEvent> event = UIValueChangeRequestEvent::create(eventNames().valuechangerequestEvent, eventInit); > > + > > + target->dispatchUIRequestEvent(event); > > Ditto. Hi Ryosuke, could you take a look again?
Comment on attachment 206341 [details] patch Assuming that patches for review since 2013 are stale, r-
Closing. This approach/spec is no longer being pursued.