Bug 34074

Summary: Convert textarea-rows-cols.html to dumpAsText()
Product: WebKit Reporter: Kent Tamura <tkent>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, bweinstein, eric, hamaji
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Proposed patch (rev.2)
none
Go back to rev1
none
ChangeLog entry was wrong...
none
ChangeLog was wrong again dimich: review+

Kent Tamura
Reported 2010-01-25 00:48:44 PST
Convert textarea-rows-cols.html to dumpAsText()
Attachments
Patch (96.54 KB, patch)
2010-01-25 00:50 PST, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Proposed patch (rev.2) (3.24 KB, patch)
2010-01-25 22:01 PST, Kent Tamura 		no flags
DetailsFormatted DiffDiff
Go back to rev1 (3.08 KB, patch)
2010-01-27 08:01 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
ChangeLog entry was wrong... (3.11 KB, patch)
2010-01-27 08:02 PST, Shinichiro Hamaji 		no flags
DetailsFormatted DiffDiff
ChangeLog was wrong again (3.11 KB, patch)
2010-02-01 15:46 PST, Shinichiro Hamaji 		dimich: review+
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Kent Tamura
Comment 1 2010-01-25 00:50:40 PST
Created attachment 47324 [details] Patch
Shinichiro Hamaji
Comment 2 2010-01-25 21:07:59 PST
Comment on attachment 47324 [details] Patch Looks good, but I feel the new test lacks information about scrollbar for the cols=1 case. It would be good to check if scrollHeight > defaultScrollHeight and scrollWidth < defaultScrollWidth ?
Kent Tamura
Comment 3 2010-01-25 22:01:51 PST
Created attachment 47387 [details] Proposed patch (rev.2)
Kent Tamura
Comment 4 2010-01-25 22:02:39 PST
(In reply to comment #2) > (From update of attachment 47324 [details]) > Looks good, but I feel the new test lacks information about scrollbar for the > cols=1 case. It would be good to check if scrollHeight > defaultScrollHeight > and scrollWidth < defaultScrollWidth ? Thank you for the comment. I added these checks.
Shinichiro Hamaji
Comment 5 2010-01-25 22:17:27 PST
Comment on attachment 47387 [details] Proposed patch (rev.2) Thanks for the fix! r+ assuming you'll address the comment below and combine patch v1 and v2 into a single patch. > -shouldBeTrue('defaultHeight > 0'); > -shouldBeTrue('defaultWidth > 0'); I guess we don't need to remove them?
Kent Tamura
Comment 6 2010-01-25 22:30:41 PST
(In reply to comment #5) > (From update of attachment 47387 [details]) > Thanks for the fix! r+ assuming you'll address the comment below and combine > patch v1 and v2 into a single patch. Wow, It seems I have missed --amend. I'll commit combined patch manually. > > -shouldBeTrue('defaultHeight > 0'); > > -shouldBeTrue('defaultWidth > 0'); > > I guess we don't need to remove them? Other assertions contain them implicitly. We may have them for readability, of course. I'll revive them.
Kent Tamura
Comment 7 2010-01-26 01:44:00 PST
Landed as r53844 <http://trac.webkit.org/changeset/53844>
Eric Seidel (no email)
Comment 9 2010-01-27 03:20:41 PST
Looks like it's been broken for a while. I suspect this change.
Shinichiro Hamaji
Comment 10 2010-01-27 08:00:07 PST
(In reply to comment #9) > Looks like it's been broken for a while. I suspect this change. Yes, this happened due to my suggestion, sorry. It seems Windows Safari doesn't show scrollbars for this case. I think the reason of this difference is the size of fonts so we can just remove checks for scrollbars.
Shinichiro Hamaji
Comment 11 2010-01-27 08:01:06 PST
Created attachment 47536 [details] Go back to rev1
Shinichiro Hamaji
Comment 12 2010-01-27 08:02:28 PST
Created attachment 47537 [details] ChangeLog entry was wrong...
Eric Seidel (no email)
Comment 13 2010-02-01 15:36:19 PST
Comment on attachment 47537 [details] ChangeLog entry was wrong... ChangeLog looks wrong.
Shinichiro Hamaji
Comment 14 2010-02-01 15:46:43 PST
Created attachment 47875 [details] ChangeLog was wrong again
Shinichiro Hamaji
Comment 15 2010-02-01 15:47:18 PST
> ChangeLog looks wrong. Oops sorry.
Dmitry Titov
Comment 16 2010-02-01 18:22:11 PST
Comment on attachment 47875 [details] ChangeLog was wrong again Looks good. r=me.
Shinichiro Hamaji
Comment 17 2010-02-01 21:10:58 PST
Committed r54201: <http://trac.webkit.org/changeset/54201>
Note You need to log in before you can comment on or make changes to this bug.