Bug 11888

Whenever the Inspector is brought up or closed, this error is printed to the console: /absolute/path/to/WebCore/bindings/objc/WebScriptObject.mm:198:[5896] JavaScript exception: SYNTAX_ERR: DOM Exception 12 Whenever "Node", "Style", "Metrics" or "Properties" is clicked, this error is printed to the console: (event handler):SYNTAX_ERR: DOM Exception 12 Using a locally-built debug build of WebKit r18334 with Safari 2.0.4 (419.3) on Mac OS X 10.4.8 (8L127).

This happens because several places in the JavaScript for both Drosera and the Web Inspector code does myElement.style.display = null;, which now throws an exception. A little investigation shows that Firefox doesn't throw an exception on this construct, while IE 7 does. We expect "myElement.style.display = null" to remove the value of the display property, effectively resetting it to its default value.

Seems kind of pointless to throw an exception in this case... sounds like we're setting ourselves up for compatibility problems if we throw and Firefox doesn't, since we'll often share the "standards" code path.

Created attachment 11943 [details] Patch

Comment on attachment 11943 [details] Patch Could we wait just a bit more before reverting a behavior that's (a) standard-compliant AND (b) matches WinIE? So far, there were no reports of Web compatibility problems (except for www.apple.com/getamac, where we share IE behavior now). To correctly remove a property, one should just set it to empty: "myElement.style.display = ''". I believe this Inspector JS bug needs to be fixed even if we decide to disable exceptions in this case, so r-.

Comment on attachment 11943 [details] Patch + Don't throw an exception when setting the value of a SSStyleDeclaration. Typo: _C_SSStyleDeclaration + if (value->isNull() || frame->settings()->shouldUseDashboardBackwardCompatibilityMode()) { It would be great to have a comment here about why we're allowing setting properties to null. r=me

Comment on attachment 11943 [details] Patch I discussed this with Alexey in #webkit, and we decided that it's best to leave the JS bindings as-is unless we come across websites relying on null resetting a property to its initial value. I'll follow up with a patch that changes the inspector and debugger's erroneous use to the correct method of using removeProperty.

Created attachment 11944 [details] Patch - fix Inspector and Drosera to do the right thing

I'm pretty nervous about this, but I guess we can wait and see. In general, the more exceptions you throw, the more likely you are to have really horrible site errors (because of buggy JS that doesn't think any exception is going to happen).

Landed in r18374.

(In reply to comment #7) > I discussed this with Alexey in #webkit, and we decided that it's best to leave > the JS bindings as-is unless we come across websites relying on null resetting > a property to its initial value. I guess we convinced each other - I now think that it would be the right thing to support setting the value to null :-). What I didn't consider earlier was that Firefox not only doesn't throw - it actually handles setting the value to null the same way as setting it to "". I thought that FF was just hiding the error, as it does for invalid values. However, this should be fixed in setProperty(), not in the JS binding code for assignment.