Bug 27800 - [HTML5][Forms] Part 4 of datalist&list: Move controlClip from RenderTextControlSingleLine to TextControlElements.cpp
: [HTML5][Forms] Part 4 of datalist&list: Move controlClip from RenderTextContr...
Status: RESOLVED WONTFIX
: WebKit
Forms
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 27247
  Show dependency treegraph
 
Reported: 2009-07-29 03:46 PST by
Modified: 2012-04-05 07:03 PST (History)


Attachments
Proposed patch (12.24 KB, patch)
2009-07-29 04:00 PST, Kent Tamura
eric: review-
Review Patch | Details | Formatted Diff | Diff
Proposed patch (rev.2) (58.40 KB, patch)
2009-08-27 23:55 PST, Kent Tamura
eric: review-
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2009-07-29 03:46:34 PST
For https://bugs.webkit.org/show_bug.cgi?id=27247 , I'd like to render a button in the border-box of the input element.  The content-box clipRect in RenderTextControlSingleLine prevents it.
So I'd like to move clipRect to RenderTextControlSingleLine::m_innerBlock.
------- Comment #1 From 2009-07-29 04:00:24 PST -------
Created an attachment (id=33710) [details]
Proposed patch
------- Comment #2 From 2009-08-04 15:59:27 PST -------
dhyatt was working on <datalist> support, he should be CCed on all bugs about it
------- Comment #3 From 2009-08-07 13:53:52 PST -------
(From update of attachment 33710 [details])
Adele Peterson is your best reviewer.  Please contact her directly.

This should be guarded by ENABLE_DATALIST, no?
------- Comment #4 From 2009-08-09 18:17:17 PST -------
(In reply to comment #3)
> This should be guarded by ENABLE_DATALIST, no?

No.  This patch is a refactoring and doesn't add new behavior though it is required to make a UI of https://bugs.webkit.org/show_bug.cgi?id=27247
------- Comment #5 From 2009-08-27 23:55:34 PST -------
Created an attachment (id=38718) [details]
Proposed patch (rev.2)

* Update test files

The code is not guarded by ENABLE(DATALIST) because this patch doesn't make any user-visible changes.
This patch can be applied without the part 3 patch.
------- Comment #6 From 2009-09-23 10:22:38 PST -------
I don't really understand what this does.  Please email Adele and ask her for a quick review, or at least a "this generally looks OK" comment from her in the bug.  This has been up for review for 3 weeks now. :(
------- Comment #7 From 2009-09-23 10:26:36 PST -------
(From update of attachment 38718 [details])
Actually, the change looks fine to me, but the ChangeLog is really missing explanation as to what you're doing here.  Please post a new patch with explanation in the Changelog as to *why* you're making this change, and little snippets next to the individual files for the tricky bits of the change.  Clear changelogs tend to improve review speeds.

http://webkit.org/coding/contributing.html#changelogs
Talks about good ChangeLogs and even points to an example.  Strongly suggested reading for new(-ish) contributors.

r- for the unclear ChangeLog, but in general this looks fine.  Again, would be good to get a "yeah this looks sane" from someone who works on forms, but if they're not answering your email (assuming you've sent one?), then there is only so much we can do. :)
------- Comment #8 From 2009-09-23 10:29:34 PST -------
(From update of attachment 38718 [details])
So all pixel test results should remain the same here?
------- Comment #9 From 2009-09-23 11:06:00 PST -------
(In reply to comment #8)
> (From update of attachment 38718 [details] [details])
> So all pixel test results should remain the same here?

That was my understanding of the patch, although tkent could comment better.
------- Comment #10 From 2012-04-05 07:03:25 PST -------
This bug is obsolete.