Bug 39936 - Implement additional DOM attribute reflection for bindings
Summary: Implement additional DOM attribute reflection for bindings
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Adler
URL:
Keywords:
Depends on: 40758 40813
Blocks:
  Show dependency treegraph
 
Reported: 2010-05-30 12:51 PDT by Darin Adler
Modified: 2010-06-18 10:55 PDT (History)
8 users (show)

See Also:


Attachments
Patch (155.17 KB, patch)
2010-05-30 13:09 PDT, Darin Adler
no flags Details | Formatted Diff | Diff
Patch (194.26 KB, patch)
2010-06-12 21:59 PDT, Darin Adler
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Adler 2010-05-30 12:51:52 PDT
Implement additional DOM attribute reflection for bindings
Comment 1 Darin Adler 2010-05-30 13:09:57 PDT
Created attachment 57425 [details]
Patch
Comment 2 WebKit Review Bot 2010-05-30 13:12:10 PDT
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.
Comment 3 WebKit Review Bot 2010-05-30 13:46:34 PDT
Attachment 57425 [details] did not build on chromium:
Build output: http://webkit-commit-queue.appspot.com/results/2687049
Comment 4 WebKit Review Bot 2010-05-30 14:31:12 PDT
Attachment 57425 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/2739037
Comment 5 Darin Adler 2010-06-12 21:59:32 PDT
Created attachment 58579 [details]
Patch
Comment 6 WebKit Review Bot 2010-06-12 22:00:23 PDT
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.
Comment 7 WebKit Review Bot 2010-06-12 22:53:22 PDT
Attachment 58579 [details] did not build on gtk:
Build output: http://webkit-commit-queue.appspot.com/results/3337025
Comment 8 Sam Weinig 2010-06-13 17:46:34 PDT
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.
Comment 9 Darin Adler 2010-06-16 10:18:50 PDT
I need to change the boolean attributes first and add a test, then land this.
Comment 10 Darin Adler 2010-06-16 10:19:27 PDT
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”.
Comment 11 Darin Adler 2010-06-16 21:18:19 PDT
Bug 40758 separates out the fix for boolean reflected attributes and includes a test for the behavior change.
Comment 12 Darin Adler 2010-06-17 22:58:06 PDT
Committed r61379: <http://trac.webkit.org/changeset/61379>
Comment 13 Kent Tamura 2010-06-17 23:19:57 PDT
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
Comment 14 WebKit Review Bot 2010-06-17 23:22:33 PDT
http://trac.webkit.org/changeset/61379 might have broken Qt Linux Release
Comment 15 Kent Tamura 2010-06-17 23:44:31 PDT
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
Comment 16 Kent Tamura 2010-06-18 00:01:34 PDT
Rolled out by r61384.
Comment 17 Darin Adler 2010-06-18 07:20:45 PDT
Strange, I did a lot of local testing. I wonder why this failed everywhere. I guess I’ll rework and land it later.
Comment 18 Darin Adler 2010-06-18 09:48:20 PDT
Committed r61413: <http://trac.webkit.org/changeset/61413>
Comment 19 Xan Lopez 2010-06-18 10:02:20 PDT
(In reply to comment #18)
> Committed r61413: <http://trac.webkit.org/changeset/61413>

This broke the GTK+ build, as predicted by EWS.
Comment 20 Darin Adler 2010-06-18 10:47:46 PDT
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.
Comment 21 Darin Adler 2010-06-18 10:55:13 PDT
<http://trac.webkit.org/changeset/61420> should fix the GTK build.