Bug 26254 - Setting white-space and word-wrap via CSS in textarea doesn't override the wrap attribute
Summary: Setting white-space and word-wrap via CSS in textarea doesn't override the wr...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: David Levin
URL: http://www.hixie.ch/tests/evil/mixed/...
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-08 05:48 PDT by Shinichiro Hamaji
Modified: 2010-05-28 13:52 PDT (History)
5 users (show)

See Also:


Attachments
Patch v1 (53.60 KB, patch)
2009-06-08 05:50 PDT, Shinichiro Hamaji
no flags Details | Formatted Diff | Diff
the expected image for layout test (112.63 KB, image/png)
2009-06-08 05:55 PDT, Shinichiro Hamaji
no flags Details
Patch v2 (339.32 KB, patch)
2009-06-25 20:07 PDT, Shinichiro Hamaji
levin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Shinichiro Hamaji 2009-06-08 05:48:40 PDT
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.
Comment 1 Shinichiro Hamaji 2009-06-08 05:50:50 PDT
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(-)
Comment 2 Shinichiro Hamaji 2009-06-08 05:52:54 PDT
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.
Comment 3 Shinichiro Hamaji 2009-06-08 05:55:14 PDT
Created attachment 31046 [details]
the expected image for layout test
Comment 4 Eric Seidel (no email) 2009-06-08 18:03:55 PDT
I would like ojan to comment here.
Comment 5 Ojan Vafai 2009-06-09 18:46:18 PDT
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.
Comment 6 Shinichiro Hamaji 2009-06-15 07:46:20 PDT
Ping? (I tried to find the reviewers on IRC but I found that it's not easy to catch them due to time difference :( )
Comment 7 Eric Seidel (no email) 2009-06-18 18:12:27 PDT
The change looks sane to me.  But really should have a stamp from Hyatt or Adele.
Comment 8 Eric Seidel (no email) 2009-06-18 18:13:52 PDT
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.
Comment 9 Eric Seidel (no email) 2009-06-25 17:33:00 PDT
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.
Comment 10 Shinichiro Hamaji 2009-06-25 20:01:06 PDT
> 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.
Comment 11 Shinichiro Hamaji 2009-06-25 20:07:13 PDT
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(-)
Comment 12 Shinichiro Hamaji 2009-06-25 20:09:30 PDT
Comment on attachment 31900 [details]
Patch v2

OK, my change for git-send-bugzilla seems to be working.
Comment 13 Eric Seidel (no email) 2009-06-26 01:47:11 PDT
Comment on attachment 31045 [details]
Patch v1

Clearing review flag on obsolete patch.
Comment 14 David Levin 2009-07-15 16:21:05 PDT
Comment on attachment 31900 [details]
Patch v2

I'm rubber stamping basically.  I'll land this and indicate that it was reviewed by eric@webkit.org

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
Comment 15 David Levin 2009-07-15 16:48:15 PDT
Committed as http://trac.webkit.org/changeset/45962
Comment 16 David Levin 2009-07-15 17:18:03 PDT
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.
Comment 17 David Levin 2009-07-15 22:35:34 PDT
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).
Comment 18 David Levin 2009-07-15 23:13:30 PDT
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.
Comment 19 Alexey Proskuryakov 2010-05-28 13:52:33 PDT
This has caused bug 39801.