getting property on prototype object must throw TypeError. Currently, WebKit returns undefined and logs a deprecation error. Chrome and Firefox throw a TypeError.
Exception is for attributes that have [LenientThis] IDL extended attribute: http://heycam.github.io/webidl/#LenientThis
<rdar://problem/24370650>
Created attachment 270005 [details] Patch
Attachment 270005 [details] did not pass style-queue: ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2059: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 51 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 270008 [details] Patch
Attachment 270008 [details] did not pass style-queue: ERROR: Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:2059: One line control clauses should not use braces. [whitespace/braces] [4] Total errors found: 1 in 29 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 270008 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270008&action=review > Source/WebCore/ChangeLog:9 > + Gettingi / Setting property on prototype object must throw TypeError as per Typo: Gettingi. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2317 > - push(@implContent, " if (UNLIKELY(!castedThis)) {\n"); > - push(@implContent, " if (jsDynamicCast<${className}Prototype*>(slotBase))\n"); > - push(@implContent, " return reportDeprecatedGetterError(*state, \"$interfaceName\", \"$name\");\n"); > - push(@implContent, " return throwGetterTypeError(*state, \"$interfaceName\", \"$name\");\n"); > - push(@implContent, " }\n"); > + push(@implContent, " if (UNLIKELY(!castedThis))\n"); > + if ($attribute->signature->extendedAttributes->{"LenientThis"}) { > + push(@implContent, " return JSValue::encode(jsUndefined());\n"); > + } else { > + push(@implContent, " return throwGetterTypeError(*state, \"$interfaceName\", \"$name\");\n"); > + } I do remember this was needed for some compat reason. Perhaps Oliver or Geoff would remember?
Comment on attachment 270008 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=270008&action=review >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2317 >> + } > > I do remember this was needed for some compat reason. > Perhaps Oliver or Geoff would remember? Alas i can't recall -- does blame help at all? It _may_ have been YUI as I recall it being a source of pain around that time.
(In reply to comment #8) > Comment on attachment 270008 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270008&action=review > > >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2317 > >> + } > > > > I do remember this was needed for some compat reason. > > Perhaps Oliver or Geoff would remember? > > Alas i can't recall -- does blame help at all? > > It _may_ have been YUI as I recall it being a source of pain around that > time. I have been going through the git log and found: https://bugs.webkit.org/show_bug.cgi?id=132626 https://bugs.webkit.org/show_bug.cgi?id=128568 I will take a look at those, but these are public sites and other browsers are throwing exceptions in this case already.
(In reply to comment #9) > (In reply to comment #8) > > Comment on attachment 270008 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=270008&action=review > > > > >> Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:2317 > > >> + } > > > > > > I do remember this was needed for some compat reason. > > > Perhaps Oliver or Geoff would remember? > > > > Alas i can't recall -- does blame help at all? > > > > It _may_ have been YUI as I recall it being a source of pain around that > > time. > > I have been going through the git log and found: > https://bugs.webkit.org/show_bug.cgi?id=132626 > https://bugs.webkit.org/show_bug.cgi?id=128568 > > I will take a look at those, but these are public sites and other browsers > are throwing exceptions in this case already. 1. One of these bugs does not mention which sites were broken :( 2. The second bug mentions making a booking on virginamerica.com. I have just tried doing so (up to the payment stage) and did not see any problem 3. The original patch was overly aggressive as we were not supposed to throw for attributes marked as [LenientThis]. This new patch takes this into consideration. 4. Since then, Chrome has also moved attributes to the prototype and is throwing in this case. Therefore, public sites would likely be broken in Chrome already if this was still a problem. Firefox has been throwing for a long time as well. 5. This was 2 years ago I propose we give this a shot again. If we notice that a particular attribute is problematic later, we can then mark it as [LenientThis] to restore the previous behavior and ask for this change to be made to the specification as well as it would not be web-compatible.
Comment on attachment 270008 [details] Patch Clearing flags on attachment: 270008 Committed r195695: <http://trac.webkit.org/changeset/195695>
All reviewed patches have been landed. Closing bug.