WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
89030
el.getAttribute('type') returns 'null' when setting el.type to null
https://bugs.webkit.org/show_bug.cgi?id=89030
Summary
el.getAttribute('type') returns 'null' when setting el.type to null
Edaena Salinas
Reported
2012-06-13 12:27:06 PDT
Created
attachment 147388
[details]
test After setting the type of a button to null using btn.type = null, btn.getAttribute('type') should return a JavaScript null and not the string 'null'.
Attachments
test
(227 bytes, text/html)
2012-06-13 12:27 PDT
,
Edaena Salinas
no flags
Details
Proposed fix and updated tests.
(4.54 KB, patch)
2012-06-13 14:31 PDT
,
Edaena Salinas
darin
: review+
Details
Formatted Diff
Diff
Proposed fix and updated tests.
(5.39 KB, patch)
2012-06-13 15:14 PDT
,
Edaena Salinas
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Edaena Salinas
Comment 1
2012-06-13 14:31:17 PDT
Created
attachment 147413
[details]
Proposed fix and updated tests.
Alexey Proskuryakov
Comment 2
2012-06-13 14:39:10 PDT
Comment on
attachment 147413
[details]
Proposed fix and updated tests. View in context:
https://bugs.webkit.org/attachment.cgi?id=147413&action=review
> Source/WebCore/ChangeLog:14 > + * html/HTMLButtonElement.cpp: > + (WebCore::HTMLButtonElement::setType): > + * html/HTMLButtonElement.h: > + (HTMLButtonElement): > + * html/HTMLButtonElement.idl:
This is a good place to explain changes in your files. For example, the change from String to AtomicString definitely needs an explanation, because it's not even part of fixing this bug. E.g. "Address another unrelated review comment from
bug 14439
."
> LayoutTests/fast/dom/HTMLButtonElement/change-type.html:64 > shouldBe("btn.getAttribute('type')", "undefined + ''");
As you are addressing unrelated feedback, you can consider changing this to "'undefined'", too.
Alexey Proskuryakov
Comment 3
2012-06-13 14:39:42 PDT
> E.g. "Address another unrelated review comment from
bug 14439
."
Better yet, "Address another unrelated review comment from
bug 14439
for slightly better performance."
Darin Adler
Comment 4
2012-06-13 14:52:55 PDT
Comment on
attachment 147413
[details]
Proposed fix and updated tests. View in context:
https://bugs.webkit.org/attachment.cgi?id=147413&action=review
>> LayoutTests/fast/dom/HTMLButtonElement/change-type.html:64 >> shouldBe("btn.getAttribute('type')", "undefined + ''"); > > As you are addressing unrelated feedback, you can consider changing this to "'undefined'", too.
I agree, you should do that.
Darin Adler
Comment 5
2012-06-13 14:59:26 PDT
Comment on
attachment 147413
[details]
Proposed fix and updated tests. For what it’s worth, I agree with all of Alexey’s comments and suggest you post a new patch dealing with them.
Edaena Salinas
Comment 6
2012-06-13 15:14:51 PDT
Created
attachment 147424
[details]
Proposed fix and updated tests.
Alexey Proskuryakov
Comment 7
2012-06-13 15:42:50 PDT
Comment on
attachment 147424
[details]
Proposed fix and updated tests. + Reviewed by NOBODY (OOPS!). For minor changes like these, you could have edited ChangeLogs to say "Reviewed by Darin Adler", and post a new patch with "commit-queue?" flag alone, no "review?" flag. + (HTMLButtonElement): Changed parameter to be AtomicString to address an unrelated + review comment from
bug 14439
for slightly better performance. We traditionally just say "Ditto" for repeated comments.
WebKit Review Bot
Comment 8
2012-06-14 01:31:43 PDT
Comment on
attachment 147424
[details]
Proposed fix and updated tests. Clearing flags on attachment: 147424 Committed
r120296
: <
http://trac.webkit.org/changeset/120296
>
WebKit Review Bot
Comment 9
2012-06-14 01:31:48 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug