Bug 39936 – Implement additional DOM attribute reflection for bindings

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Darin Adler]

Implement additional DOM attribute reflection for bindings

[Darin Adler]

Created attachment 57425 [details]
Patch

[WebKit Review Bot]

Attachment 57425 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:

WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:529:  _g_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:535:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:537:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:545:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:547:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:549:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:550:  _g_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:556:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:558:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:560:  Extra space before )  [whitespace/parens] [2]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:566:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:568:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:575:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:577:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:579:  Extra space before )  [whitespace/parens] [2]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:585:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:587:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:594:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:596:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:604:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:606:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:608:  Extra space before ( in function call  [whitespace/parens] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:609:  _g_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 60 in 72 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 57425 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2687049

[WebKit Review Bot]

Attachment 57425 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2739037

[Darin Adler]

Created attachment 58579 [details]
Patch

[WebKit Review Bot]

Attachment 58579 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
er names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:90:  converted_obj_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:113:  converted_str_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:114:  converted_obj_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:116:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:128:  converted_str_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:129:  converted_obj_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:131:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:143:  converted_str_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:144:  converted_obj_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:147:  g_res is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:163:  converted_serialized_arg is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:402:  converted_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:422:  converted_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:442:  converted_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:495:  converted_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:514:  converted_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
WebCore/bindings/scripts/test/GObject/WebKitDOMTestObj.cpp:567:  converted_value is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
Total errors found: 24 in 76 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[WebKit Review Bot]

Attachment 58579 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3337025

[Sam Weinig]

Comment on attachment 58579 [details]
Patch

r=me. My only concern is with the change to boolean reflected attributes. Is the change to not set the name as the value detectable in anyway (eg. doing a getAttribute())? If so, we should add tests for that.

[Darin Adler]

I need to change the boolean attributes first and add a test, then land this.

[Darin Adler]

Comment on attachment 58579 [details]
Patch

I’m going to clear Sam’s review flag so this doesn’t show up as a “reviewed patch”.

[Darin Adler]

Bug 40758 separates out the fix for boolean reflected attributes and includes a test for the behavior change.

[Darin Adler]

Committed r61379: <http://trac.webkit.org/changeset/61379>

[Kent Tamura]

This change broke the followings on Chromium Linux (maybe all platforms)

Regressions: Unexpected text diff mismatch :
  fast/dom/element-attribute-js-null.html = TEXT
  fast/dom/image-object.html = TEXT
  fast/events/onload-after-document-close-with-subresource.html = TEXT
  fast/forms/mutation-event-recalc.html = TEXT
  fast/forms/option-value-and-label.html = TEXT
  fast/js/custom-constructors.html = TEXT
  http/tests/misc/image-blocked-src-change.html = TEXT
  http/tests/misc/image-checks-for-accept.html = TEXT
  http/tests/misc/image-error.html = TEXT
  http/tests/security/frame-loading-via-document-write.html = TEXT
  http/tests/security/local-image-from-remote-whitelisted.html = TEXT
  http/tests/xmlhttprequest/xmlhttprequest-image-not-loaded.html = TEXT

Regressions: Unexpected image mismatch :
  fast/canvas/image-object-in-canvas.html = IMAGE
  fast/canvas/image-pattern-rotate.html = IMAGE

Regressions: Unexpected image and text mismatch : 
  editing/pasteboard/select-element-1.html = IMAGE+TEXT

[WebKit Review Bot]

http://trac.webkit.org/changeset/61379 might have broken Qt Linux Release

[Kent Tamura]

The following tests failed on Leopard Intel Release:

editing/pasteboard/select-element-1.html -> failed
fast/canvas/webgl/tex-image-and-sub-image-2d-with-image.html -> failed
fast/dom/element-attribute-js-null.html -> failed
fast/dom/image-object.html -> failed
fast/events/onload-after-document-close-with-subresource.html -> failed
fast/forms/mutation-event-recalc.html -> failed
fast/forms/option-value-and-label.html -> failed
fast/js/custom-constructors.html -> failed
http/tests/misc/acid3.html -> failed
http/tests/misc/image-blocked-src-change.html -> failed
http/tests/misc/image-error.html -> failed
http/tests/security/local-image-from-remote-whitelisted.html -> failed
http/tests/xmlhttprequest/xmlhttprequest-image-not-loaded.html -> failed

[Kent Tamura]

Rolled out by r61384.

[Darin Adler]

Strange, I did a lot of local testing. I wonder why this failed everywhere. I guess I’ll rework and land it later.

[Darin Adler]

Committed r61413: <http://trac.webkit.org/changeset/61413>

[Xan Lopez]

(In reply to comment #18)
> Committed r61413: <http://trac.webkit.org/changeset/61413>

This broke the GTK+ build, as predicted by EWS.

[Darin Adler]

The GTK failure on EWS was a different one in an earlier version of the patch. I fixed that one.

I have a few minutes and I’ll try to fix the new failure now.

[Darin Adler]

<http://trac.webkit.org/changeset/61420> should fix the GTK build.