Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(76)

Issue 795173003: Stricter parsing for rows/cols attribute on textarea (Closed)

Created:
6 years ago by rwlbuis
Modified:
6 years ago
Reviewers:
tkent
CC:
blink-reviews, dglazkov+blink, blink-reviews-html_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Project:
blink
Visibility:
Public.

Description

Stricter parsing for rows/cols attribute on textarea According to the latest spec "If the cols attribute is specified, its value must be a valid non-negative integer greater than zero. If applying the rules for parsing non-negative integers to the attribute's value results in a number greater than zero, then the element's character width is that value; otherwise, it is 20." The rows attribute is similar but the default value is 2. With this patch parsing is more strict and above rules are honored. This was showing as failures here, fixed with this patch: http://w3c-test.org/html/dom/reflection-forms.html Behavior matches Firefox. Spec reference: https://html.spec.whatwg.org/multipage/forms.html#attr-textarea-cols Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187282

Patch Set 1 #

Patch Set 2 : Remove unrelated change #

Patch Set 3 : Add tests #

Patch Set 4 : Add some subtests I forgot... #

Total comments: 1

Patch Set 5 : Fix test result #

Total comments: 2

Patch Set 6 : Patch for landing #

Unified diffs Side-by-side diffs Delta from patch set Stats (+166 lines, -4 lines) Patch
A LayoutTests/fast/dom/HTMLTextAreaElement/cols-attribute.html View 1 2 3 4 5 1 chunk +50 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/HTMLTextAreaElement/cols-attribute-expected.txt View 1 2 3 1 chunk +31 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/HTMLTextAreaElement/rows-attribute.html View 1 2 3 4 5 1 chunk +50 lines, -0 lines 0 comments Download
A LayoutTests/fast/dom/HTMLTextAreaElement/rows-attribute-expected.txt View 1 2 3 4 1 chunk +31 lines, -0 lines 0 comments Download
M Source/core/html/HTMLTextAreaElement.cpp View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 14 (5 generated)
rwlbuis
PTAL. This is a lot like my last patch but with more tests this time.
6 years ago (2014-12-12 23:07:48 UTC) #2
tkent
https://codereview.chromium.org/795173003/diff/60001/Source/core/dom/Document.cpp File Source/core/dom/Document.cpp (right): https://codereview.chromium.org/795173003/diff/60001/Source/core/dom/Document.cpp#newcode2393 Source/core/dom/Document.cpp:2393: Element* documentElement = this->documentElement(); Is this related to the ...
6 years ago (2014-12-15 01:09:29 UTC) #3
rwlbuis
On 2014/12/15 01:09:29, unavailable until Feburuary wrote: > https://codereview.chromium.org/795173003/diff/60001/Source/core/dom/Document.cpp > File Source/core/dom/Document.cpp (right): > > ...
6 years ago (2014-12-15 13:47:09 UTC) #4
rwlbuis
On 2014/12/15 13:47:09, rwlbuis wrote: > On 2014/12/15 01:09:29, unavailable until Feburuary wrote: > > ...
6 years ago (2014-12-15 19:38:55 UTC) #6
tkent
lgtm https://codereview.chromium.org/795173003/diff/100001/LayoutTests/fast/dom/HTMLTextAreaElement/script-tests/cols-attribute.js File LayoutTests/fast/dom/HTMLTextAreaElement/script-tests/cols-attribute.js (right): https://codereview.chromium.org/795173003/diff/100001/LayoutTests/fast/dom/HTMLTextAreaElement/script-tests/cols-attribute.js#newcode1 LayoutTests/fast/dom/HTMLTextAreaElement/script-tests/cols-attribute.js:1: description("HTMLTextAreaElement cols attribute test"); Please merge this into ...
6 years ago (2014-12-15 23:51:56 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/795173003/120001
6 years ago (2014-12-16 15:13:52 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: win_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/42074)
6 years ago (2014-12-16 16:55:27 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/795173003/120001
6 years ago (2014-12-16 17:39:41 UTC) #13
commit-bot: I haz the power
6 years ago (2014-12-16 18:40:06 UTC) #14
Message was sent while issue was closed.
Committed patchset #6 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187282

Powered by Google App Engine
This is Rietveld 408576698