Bug 26254 – Setting white-space and word-wrap via CSS in textarea doesn't override the wrap attribute

• Component: CSS
• Status: RESOLVED FIXED

[Shinichiro Hamaji]

Reproduce:

1. Access http://www.hixie.ch/tests/evil/mixed/wraptextarea.html
2. Check three textareas in the HTML page if their look matches with their descriptions.

Note that the patch I'll post will fix most issues in the HTML page, but I left an issue with which line breaks in white-space:normal textareas are not collapsed. I believe this bug is the bug of somewhere else, and I want to leave the bug for now. I'll open another bug for this issue once this bug is closed.

[Shinichiro Hamaji]

Created attachment 31045 [details]
Patch v1

 LayoutTests/ChangeLog                              |   18 +
 LayoutTests/fast/forms/basic-textareas.html        |   17 +-
 .../fast/forms/basic-textareas-expected.checksum   |    2 +-
 .../mac/fast/forms/basic-textareas-expected.png    |  Bin 120454 -> 115333 bytes
 .../mac/fast/forms/basic-textareas-expected.txt    |  400 +++++++++++++++-----
 WebCore/ChangeLog                                  |   19 +
 WebCore/css/html4.css                              |    2 +
 WebCore/html/HTMLTextAreaElement.cpp               |   10 +
 WebCore/rendering/RenderTextControlMultiLine.cpp   |   13 -
 9 files changed, 367 insertions(+), 114 deletions(-)

[Shinichiro Hamaji]

Comment on attachment 31045 [details]
Patch v1

Note that the whitespace I inserted into 'Lorem ipsum dolor' in layout test is intentional. We can see if whitespaces are collapsed by this.

[Shinichiro Hamaji]

Created attachment 31046 [details]
the expected image for layout test

[Eric Seidel (no email)]

I would like ojan to comment here.

[Ojan Vafai]

Hamaji and I wrote the patch together, so it looks good to me. :) Adele, Hyatt, the code change here is just ~20 lines if you have a minute to review this. No rush though, nothing is blocking on this.

[Shinichiro Hamaji]

Ping? (I tried to find the reviewers on IRC but I found that it's not easy to catch them due to time difference :( )

[Eric Seidel (no email)]

The change looks sane to me.  But really should have a stamp from Hyatt or Adele.

[Eric Seidel (no email)]

Comment on attachment 31045 [details]
Patch v1

Actually... This change looks fine to me.  Adele or Hyatt should feel free to r- if for some reason I'm wrong, but I'd rather take action than leave this sitting for another 10 days.

[Eric Seidel (no email)]

curl "https://bugs.webkit.org/attachment.cgi?id=31045" | svn-apply
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100 54883  100 54883    0     0   111k      0 --:--:-- --:--:-- --:--:-- 4466k
Failed to find 'Index:' in:
-------------------------------------------------------------------
e382983a97080e60521e65ed498e62209ea2f8af

-------------------------------------------------------------------
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/forms/basic-textareas.html
patching file LayoutTests/platform/mac/fast/forms/basic-textareas-expected.checksum
patch: **** Only garbage was found in the patch input.
patch -p0 "LayoutTests/platform/mac/fast/forms/basic-textareas-expected.png" returned 2.  Pass --force to ignore patch failures.

Looks like it's a git patch w/o any content for the binary files.  svn-create-patch is the proper way to create patches.  If you need to include binary content from git, you'll need to use git format-patch.

Someone can apply this manually with a little work.

[Shinichiro Hamaji]

> Looks like it's a git patch w/o any content for the binary files. 
> svn-create-patch is the proper way to create patches.  If you need to include
> binary content from git, you'll need to use git format-patch.
Thanks for trying to land this patch!

Sorry, I'll create the patch again with a binary. I'm using git-send-bugzilla script. I'm guessing adding --binary for the second diff-tree would work. It would be nice if git-send-bugzilla has an option which adds --binary for people who don't have committer access.

[Shinichiro Hamaji]

Created attachment 31900 [details]
Patch v2

 LayoutTests/ChangeLog                              |   18 +
 LayoutTests/fast/forms/basic-textareas.html        |   17 +-
 .../fast/forms/basic-textareas-expected.checksum   |    2 +-
 .../mac/fast/forms/basic-textareas-expected.png    |  Bin 120454 -> 115333 bytes
 .../mac/fast/forms/basic-textareas-expected.txt    |  400 +++++++++++++++-----
 WebCore/ChangeLog                                  |   19 +
 WebCore/css/html4.css                              |    2 +
 WebCore/html/HTMLTextAreaElement.cpp               |   10 +
 WebCore/rendering/RenderTextControlMultiLine.cpp   |   13 -
 9 files changed, 367 insertions(+), 114 deletions(-)

[Shinichiro Hamaji]

Comment on attachment 31900 [details]
Patch v2

OK, my change for git-send-bugzilla seems to be working.

[Eric Seidel (no email)]

Comment on attachment 31045 [details]
Patch v1

Clearing review flag on obsolete patch.

[David Levin]

Comment on attachment 31900 [details]
Patch v2

I'm rubber stamping basically.  I'll land this and indicate that it was reviewed by [e-mail]

The only difference between the current patch and the one he r+'ed is that this one has the image: LayoutTests/platform/mac/fast/forms/basic-textareas-expected.png

[David Levin]

Committed as http://trac.webkit.org/changeset/45962

[David Levin]

http://trac.webkit.org/changeset/45963

I had to revert the change because several layout tests failed (and it was my mistake that I committed this too fast once I got it applied to my tree).

While reverting, I found that somehow what I submitted omitted the html4.css change, so perhaps that was the problem.

I try this again later tonight in a better manner.

[David Levin]

Committed as http://trac.webkit.org/changeset/45966
with the html.css this time (and after doing a build and passing all tests -- which I mistakenly omitted last time).

[David Levin]

Here's the commit for the windows expected result fix up:
http://trac.webkit.org/changeset/45967

It didn't get a new png because I pulled it from the build machine, and this is all that is exposed there.

[Alexey Proskuryakov]

This has caused bug 39801.