Bug 31821 - Type=number UI part 1: pseudo classes and appearances
Summary: Type=number UI part 1: pseudo classes and appearances
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Kent Tamura
URL:
Keywords:
Depends on:
Blocks: 27968 32813
  Show dependency treegraph
 
Reported: 2009-11-23 22:34 PST by Kent Tamura
Modified: 2009-12-20 22:47 PST (History)
4 users (show)

See Also:


Attachments
Proposed patch (rev.2) (14.62 KB, patch)
2009-11-23 23:49 PST, Kent Tamura
abarth: review-
Details | Formatted Diff | Diff
A figure explaining spin button layout (19.19 KB, image/png)
2009-12-16 23:08 PST, Kent Tamura
no flags Details
Proposed patch (rev.3) (14.59 KB, patch)
2009-12-17 00:28 PST, Kent Tamura
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2009-11-23 22:34:30 PST
To implement <input type=number> UI, introduces
   - ::-webkit-inner-spin-button pseudo CSS selector
   - ::-webkit-outer-spin-button pseudo CSS selector
   - new appearance type: inner-spin-button
   - new appearance type: outer-spin-button

You can see examples in Bug#27968 .  We'll have two types; inner and outer, because Windows and Mac have different manners for such controls.
Comment 1 Kent Tamura 2009-11-23 22:36:40 PST
Comments on this part in Bug#27968:

■■ Comment #12 From Eric Seidel 2009-09-23 10:31:35 PST (-) [reply] 
(From update of attachment 39537 [details])
The change looks OK.  Where did the "*-spin-button" names come from?  What does
it have to do with spinning?  Can you post a picture of what these inputs are
going to look like?

■■ Comment #15 From Eric Seidel 2009-09-23 10:36:37 PST (-) [reply] 
Why are these called spin buttons if the Mac APIs call them steppers?  Does
windows call these "Spin buttons"?

■■ Comment #16 From Eric Seidel 2009-09-23 10:38:18 PST (-) [reply] 
Why are they called inner and outer spin buttons if they're really up and down
steppers?  Please cite the spec, or talk about where these names came from. 
Ideally in your ChangeLogs, since that's what reviewers read.
Comment 2 Kent Tamura 2009-11-23 23:49:51 PST
Created attachment 43751 [details]
Proposed patch (rev.2)

* Improve ChangeLog
* Resolve conflicts

(In reply to comment #1)
> Comments on this part in Bug#27968:
> 
> ■■ Comment #12 From Eric Seidel 2009-09-23 10:31:35 PST (-) [reply] 
> (From update of attachment 39537 [details] [details])
> The change looks OK.  Where did the "*-spin-button" names come from?  What does
> it have to do with spinning?  Can you post a picture of what these inputs are
> going to look like?
> 
> ■■ Comment #15 From Eric Seidel 2009-09-23 10:36:37 PST (-) [reply] 
> Why are these called spin buttons if the Mac APIs call them steppers?  Does
> windows call these "Spin buttons"?

Right.  Windows and GTK call such controls "spin buttons."
I don't have strong preference about the name.  However "Stepper" is Mac-only uncommon.

> ■■ Comment #16 From Eric Seidel 2009-09-23 10:38:18 PST (-) [reply] 
> Why are they called inner and outer spin buttons if they're really up and down
> steppers?  Please cite the spec, or talk about where these names came from. 
> Ideally in your ChangeLogs, since that's what reviewers read.

An inner or outer spin button represents a pair of a upper arrow button and a lower arrow button.
An inner-spin-button will be placed at the inside of a text field (Windows' standard style), and
an outer-spin-button will be placed at the outside of a text field (Mac's standard style).

I added some sentences about this to ChangeLog.
Comment 3 Eric Seidel 2009-11-24 10:03:07 PST
Comment on attachment 43751 [details]
Proposed patch (rev.2)

It's not at all clear from the code that "inner" means linux/win and "outer" means mac.  Can we clear this up some how?  I'm really not sure I understand why the implementations need separate CSS styles like this.

I'm just still confused why this patch does what it does.
Comment 4 Adam Barth 2009-11-30 12:37:03 PST
style-queue successfully ran check-webkit-style on attachment 43751 [details] without any errors
Comment 5 WebKit Review Bot 2009-12-16 01:46:19 PST
Attachment 43751 [details] did not pass chromium-ews:
Full output: http://webkit-commit-queue.appspot.com/results/128468
Comment 6 Adam Barth 2009-12-16 07:43:11 PST
Comment on attachment 43751 [details]
Proposed patch (rev.2)

This patch has been in the review queue for a while and does not appear to build.  If you're still interested in this patch, please post an updated version that compiles.
Comment 7 Kent Tamura 2009-12-16 23:08:48 PST
Created attachment 45038 [details]
A figure explaining spin button layout

(In reply to comment #3)
> (From update of attachment 43751 [details])
> It's not at all clear from the code that "inner" means linux/win and "outer"
> means mac.  Can we clear this up some how?  I'm really not sure I understand
> why the implementations need separate CSS styles like this.
> 
> I'm just still confused why this patch does what it does.

"Inner" means that it will be laid on the inside of text field border, and "outer" means it will be laid on the outside of text field border. Please see the figure I attach.
If we introduced just one spin-button class, we would need to implement two different layout and switch them by any way [1]. I thought introducing two classes, doing layout for both, 0x0 size for either, was much simpler than switching.

[1] may be
 * A renderer has "#if PLATFORM(...)"
  This seems inappropriate.  I don't know which layout is suitable for platforms other than mac and win. 
 * A platform code provides a flag representing inner or outer, a renderer switches layout by the flag.
  This may be another reasonable solution.
Comment 8 Kent Tamura 2009-12-16 23:10:28 PST
(In reply to comment #5)
> Attachment 43751 [details] did not pass chromium-ews:
> Full output: http://webkit-commit-queue.appspot.com/results/128468

Ah, the patch had a typo for !NEW_THEME code path.
Good catch by EWS!
Comment 9 Kent Tamura 2009-12-17 00:28:34 PST
Created attachment 45042 [details]
Proposed patch (rev.3)

- Fix build error without NEW_THEME
Comment 10 WebKit Review Bot 2009-12-17 00:31:38 PST
style-queue ran check-webkit-style on attachment 45042 [details] without any errors.
Comment 11 Darin Adler 2009-12-19 11:21:14 PST
Comment on attachment 45042 [details]
Proposed patch (rev.3)

I didn't carefully review the names of the appearance values, but this all seems OK. r=me on landing this next step.
Comment 12 WebKit Commit Bot 2009-12-20 20:58:07 PST
Comment on attachment 45042 [details]
Proposed patch (rev.3)

Clearing flags on attachment: 45042

Committed r52432: <http://trac.webkit.org/changeset/52432>
Comment 13 WebKit Commit Bot 2009-12-20 20:58:14 PST
All reviewed patches have been landed.  Closing bug.