Setter HTMLMeterElement's attributes to non-finite values throws wrong exception type. It should throw a TypeError but we throw a NOT_SUPPORTED_ERR.
Created attachment 289486 [details] Patch
Comment on attachment 289486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289486&action=review That iOS Simulator error looks like it might be a real one. Please investigate and fix. > Source/WebCore/html/HTMLMeterElement.cpp:88 > setAttributeWithoutSynchronization(minAttr, AtomicString::number(min)); This looks the same as "[Reflect]" but for the setter only; is there some way to use that here? Maybe we can add a way to state "reflect for setter, but call C++ for getter" with extended attributes? Maybe "[ReflectSetterOnly]"? > Source/WebCore/html/HTMLMeterElement.cpp:98 > setAttributeWithoutSynchronization(maxAttr, AtomicString::number(max)); Ditto. > Source/WebCore/html/HTMLMeterElement.cpp:109 > setAttributeWithoutSynchronization(valueAttr, AtomicString::number(value)); Ditto. > Source/WebCore/html/HTMLMeterElement.cpp:120 > setAttributeWithoutSynchronization(lowAttr, AtomicString::number(low)); Ditto. > Source/WebCore/html/HTMLMeterElement.cpp:131 > setAttributeWithoutSynchronization(highAttr, AtomicString::number(high)); Ditto. > Source/WebCore/html/HTMLMeterElement.cpp:142 > setAttributeWithoutSynchronization(optimumAttr, AtomicString::number(optimum)); Ditto.
(In reply to comment #2) > Comment on attachment 289486 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289486&action=review > > That iOS Simulator error looks like it might be a real one. Please > investigate and fix. > > > Source/WebCore/html/HTMLMeterElement.cpp:88 > > setAttributeWithoutSynchronization(minAttr, AtomicString::number(min)); > > This looks the same as "[Reflect]" but for the setter only; is there some > way to use that here? Maybe we can add a way to state "reflect for setter, > but call C++ for getter" with extended attributes? Maybe > "[ReflectSetterOnly]"? > > > Source/WebCore/html/HTMLMeterElement.cpp:98 > > setAttributeWithoutSynchronization(maxAttr, AtomicString::number(max)); > > Ditto. > > > Source/WebCore/html/HTMLMeterElement.cpp:109 > > setAttributeWithoutSynchronization(valueAttr, AtomicString::number(value)); > > Ditto. > > > Source/WebCore/html/HTMLMeterElement.cpp:120 > > setAttributeWithoutSynchronization(lowAttr, AtomicString::number(low)); > > Ditto. > > > Source/WebCore/html/HTMLMeterElement.cpp:131 > > setAttributeWithoutSynchronization(highAttr, AtomicString::number(high)); > > Ditto. > > > Source/WebCore/html/HTMLMeterElement.cpp:142 > > setAttributeWithoutSynchronization(optimumAttr, AtomicString::number(optimum)); > > Ditto. We would probably need a [ReflectSetterOnly] as you suggest. Although, I have not checked yet if the behavior of our getters is actually consistent with other browsers (it could be that [Reflect] is the right thing to do). Note however that our bindings generator currently does not even seem to have support for [Reflect] for floating point attributes. This should probably be added.
Comment on attachment 289486 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=289486&action=review >>> Source/WebCore/html/HTMLMeterElement.cpp:88 >>> setAttributeWithoutSynchronization(minAttr, AtomicString::number(min)); >> >> This looks the same as "[Reflect]" but for the setter only; is there some way to use that here? Maybe we can add a way to state "reflect for setter, but call C++ for getter" with extended attributes? Maybe "[ReflectSetterOnly]"? > > We would probably need a [ReflectSetterOnly] as you suggest. Although, I have not checked yet if the behavior of our getters is actually consistent with other browsers (it could be that [Reflect] is the right thing to do). > > Note however that our bindings generator currently does not even seem to have support for [Reflect] for floating point attributes. This should probably be added. To state the obvious, I am guessing that code generation for [Reflect] for floating point attributes is almost identical to [Reflect] for integer attributes.
Comment on attachment 289486 [details] Patch Attachment 289486 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/2120982 New failing tests: imported/w3c/web-platform-tests/html/semantics/forms/the-meter-element/meter.html
Created attachment 289490 [details] Archive of layout-test-results from ews123 for ios-simulator-elcapitan-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-elcapitan-wk2 Platform: Mac OS X 10.11.6
Committed r206243: <http://trac.webkit.org/changeset/206243>
(In reply to comment #7) > Committed r206243: <http://trac.webkit.org/changeset/206243> iOS just needed a rebaseline for the new PASS.
(In reply to comment #2) > Comment on attachment 289486 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=289486&action=review > > That iOS Simulator error looks like it might be a real one. Please > investigate and fix. > > > Source/WebCore/html/HTMLMeterElement.cpp:88 > > setAttributeWithoutSynchronization(minAttr, AtomicString::number(min)); > > This looks the same as "[Reflect]" but for the setter only; is there some > way to use that here? Maybe we can add a way to state "reflect for setter, > but call C++ for getter" with extended attributes? Maybe > "[ReflectSetterOnly]"? I checked that the behavior of our getters is spec-compliant and matches Chrome at least. Therefore, this is not really reflection and [Reflect] is not suitable. As you say, we could add [ReflectSetterOnly] but I am not sure it is worth it. I would also find it a little bit confusing because it is not really a reflection if only the setter is "transparent".