HTMLOutputElement::htmlFor should be readonly

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Kentaro Hara]

Spec: http://dev.w3.org/html5/spec-preview/the-output-element.html

The spec says htmlFor should be readonly, but WebKit implements it as no-readonly.

htmlFor is implemented in http://trac.webkit.org/changeset/71373

bashi, tkent: Is there any reason why htmlFor is implemented as no-readonly?

[Kent Tamura]

I think htmlFor was not read-only in old drafts.

[Kenichi Ishibashi]

I don't remember, but I think it wasn't readonly in old spec, too. I think we should make it readonly.

Please feel free to prepare the patch or assign the bug to me.

[Vineet Chaudhary (vineetc)]

Created attachment 184331 [details]
patch

[Kent Tamura]

Comment on attachment 184331 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184331&action=review

> Source/WebCore/ChangeLog:16
> +        * bindings/js/JSHTMLOutputElementCustom.cpp: Removed setHtmlFor() method.
> +        * bindings/v8/custom/V8HTMLOutputElementCustom.cpp: Removed htmlForAccessorSetter().

I think we can remove htmlFor() too.

[Kent Tamura]

Comment on attachment 184331 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184331&action=review

>> Source/WebCore/ChangeLog:16
>> +        * bindings/v8/custom/V8HTMLOutputElementCustom.cpp: Removed htmlForAccessorSetter().
> 
> I think we can remove htmlFor() too.

by removing [Custom] in the IDL.

[Vineet Chaudhary (vineetc)]

(In reply to comment #5)
> (From update of attachment 184331 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184331&action=review
> 
> >> Source/WebCore/ChangeLog:16
> >> +        * bindings/v8/custom/V8HTMLOutputElementCustom.cpp: Removed htmlForAccessorSetter().
> > 
> > I think we can remove htmlFor() too.
> 
> by removing [Custom] in the IDL.

Yes I just realized that we can remove custom bindings here thanks for pointing,
I will upload patch again.

[Kentaro Hara]

Comment on attachment 184331 [details]
patch

Thanks for fixing the bug. Would you check the behavior of other browsers (Firefox, Opera, IE)?

[Vineet Chaudhary (vineetc)]

Created attachment 184384 [details]
updated_patch

(In reply to comment #7)
> (From update of attachment 184331 [details])
> Thanks for fixing the bug. Would you check the behavior of other browsers (Firefox, Opera, IE)?

I checked the behavior with other browsers:

Read/Writable On
IE      : 8.0
Opera   : 12.12 (Ubuntu)

Readonly On
Firefox : 18.0

[Kentaro Hara]

Comment on attachment 184384 [details]
updated_patch

Given that Firefox follows the spec behavior, the change looks reasonable. Thanks for removing the custom bindings. (The original motivation that I filed this bug is that I wanted to remove the custom bindings.)

[WebKit Review Bot]

Comment on attachment 184384 [details]
updated_patch

Rejecting attachment 184384 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 184384, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
LOutputElementCustom.cpp'
patching file Source/WebCore/html/HTMLOutputElement.idl
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/dom/HTMLOutputElement/dom-settable-token-list-expected.txt
patching file LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kentaro Hara']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16072854

[Vineet Chaudhary (vineetc)]

Created attachment 184538 [details]
patch_for_landing

Sorry for previous patch it failed to apply.
Attaching same patch rebaselined.

[Vineet Chaudhary (vineetc)]

(In reply to comment #10)
> (From update of attachment 184384 [details])
> Rejecting attachment 184384 [details] from commit-queue.
> 
> Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 184384, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue
> 
> Last 500 characters of output:
> LOutputElementCustom.cpp'
> patching file Source/WebCore/html/HTMLOutputElement.idl
> patching file LayoutTests/ChangeLog
> Hunk #1 succeeded at 1 with fuzz 3.
> patching file LayoutTests/fast/dom/HTMLOutputElement/dom-settable-token-list-expected.txt
> patching file LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js
> 
> Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kentaro Hara']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue
> 
> Full output: http://queues.webkit.org/results/16072854

Hi haraken,
I am able to apply patch locally but not sure why it is failing for bot.
Could you please point if I am missing something.

[Vineet Chaudhary (vineetc)]

Created attachment 184549 [details]
Patch

[Kentaro Hara]

Did you rebase your patch with the latest WebKit trunk?

[Vineet Chaudhary (vineetc)]

(In reply to comment #14)
> Did you rebase your patch with the latest WebKit trunk?

Yes I did that too also tried with svn-create-patch and webkit-patch both.

[Kentaro Hara]

Comment on attachment 184549 [details]
Patch

Hmm, then I wonder why bots are purple (It means the bots failed in applying your patch). Let's try to commit anyway.

[WebKit Review Bot]

Comment on attachment 184549 [details]
Patch

Rejecting attachment 184549 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 184549, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
LOutputElementCustom.cpp'
patching file Source/WebCore/html/HTMLOutputElement.idl
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/dom/HTMLOutputElement/dom-settable-token-list-expected.txt
patching file LayoutTests/fast/dom/HTMLOutputElement/script-tests/dom-settable-token-list.js

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force', '--reviewer', 'Kentaro Hara']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16082923

[Kent Tamura]

(In reply to comment #13)
> Created an attachment (id=184549) [details]
> Patch

Looks like end-of-line code of some files are CR-LF.

[Vineet Chaudhary (vineetc)]

Created attachment 184810 [details]
Patch

[Vineet Chaudhary (vineetc)]

(In reply to comment #19)
> Created an attachment (id=184810) [details]
> Patch

(In reply to comment #18)
> (In reply to comment #13)
> > Created an attachment (id=184549) [details] [details]
> > Patch
> 
> Looks like end-of-line code of some files are CR-LF.

Thanks tkent it helped!! Editing vcxproj file in visual studio/windows helps.

[Build Bot]

Comment on attachment 184810 [details]
Patch

Attachment 184810 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16123515

[Build Bot]

Comment on attachment 184810 [details]
Patch

Attachment 184810 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16117579

New failing tests:
fast/dom/HTMLOutputElement/dom-settable-token-list.html

[WebKit Review Bot]

Comment on attachment 184810 [details]
Patch

Attachment 184810 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16119545

New failing tests:
fast/dom/HTMLOutputElement/dom-settable-token-list.html

[Build Bot]

Comment on attachment 184810 [details]
Patch

Attachment 184810 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16117711

New failing tests:
fast/dom/HTMLOutputElement/dom-settable-token-list.html

[Vineet Chaudhary (vineetc)]

Created attachment 185082 [details]
another_try

It seems patch failing to apply for WIN only.
I checked other patches includes *.vcxproj files and those also fails to apply patch for windows.
Can/should we land this patch manually? //Apologize for previous wrong patch.

[Kentaro Hara]

Comment on attachment 185082 [details]
another_try

Maybe it is a problem of CR+LF? Anyway the patch looks OK, so please feel free to try to land it manually, keeping watching the tree.

[WebKit Review Bot]

Comment on attachment 185082 [details]
another_try

Clearing flags on attachment: 185082

Committed r141063: <http://trac.webkit.org/changeset/141063>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.