Bug 56001 - <meter> should only support horizontal indicator
Summary: <meter> should only support horizontal indicator
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on: 57564
Blocks: 50661
  Show dependency treegraph
 
Reported: 2011-03-09 01:03 PST by Hajime Morrita
Modified: 2011-04-01 09:52 PDT (History)
9 users (show)

See Also:


Attachments
Patch (456.68 KB, patch)
2011-03-10 00:39 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (442.65 KB, patch)
2011-03-30 19:51 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (443.04 KB, patch)
2011-03-31 09:27 PDT, Hajime Morrita
no flags Details | Formatted Diff | Diff
Added skip list entries that need expectation update. (445.70 KB, patch)
2011-04-01 08:47 PDT, Hajime Morrita
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2011-03-09 01:03:27 PST
Current <meter> change its appearance based on its dimension.
But such kind of style-determined-by-layout behavior is tricky to implement 
and may (and did) cause some layout instability.
(See https://lists.webkit.org/pipermail/webkit-dev/2011-March/016140.html)

Instead of this dynamic appearance switching, WebKit should provide the way to
choose the visual direction by styles.

Because text-direction is used to switch its visual between rtl and ltr, 
writing-mode looks a good candidate for the knob.

Why don't provide two separate -webkit-appearance (like meter-vertical and meter-horizontal)?
Because WebKit allows user-styling for form controls via pseudo classes like -webkit-meter, 
and this user-style is enabled by specifying "-webkit-appearance: none". 
Here we already use -webkit-appearance, thus cannot give meter-vertical appearance for it.
Comment 1 Dimitri Glazkov (Google) 2011-03-09 10:40:55 PST
What do you think about adding a whole new CSS property? writing-mode still seems to say not quite what we mean.
Comment 2 Simon Fraser (smfr) 2011-03-09 10:49:30 PST
I don't think writing-mode is the right property to use.
Comment 3 Hajime Morrita 2011-03-10 00:39:26 PST
Created attachment 85292 [details]
Patch
Comment 4 Hajime Morrita 2011-03-10 00:40:33 PST
Hi Dimitri, could you take a look?
This is a preparation for making <meter> new-shadowish.
Comment 5 Dimitri Glazkov (Google) 2011-03-10 14:28:47 PST
(In reply to comment #4)
> Hi Dimitri, could you take a look?
> This is a preparation for making <meter> new-shadowish.

Given that Simon also thinks that writing-mode is the right approach, perhaps we should have a quick discussion first.
Comment 6 Dimitri Glazkov (Google) 2011-03-10 14:28:58 PST
(In reply to comment #5)
> (In reply to comment #4)
> > Hi Dimitri, could you take a look?
> > This is a preparation for making <meter> new-shadowish.
> 
> Given that Simon also thinks that writing-mode is NOT the right approach, perhaps we should have a quick discussion first.
Comment 7 Dave Hyatt 2011-03-10 14:47:15 PST
Yeah writing-mode is really not the correct approach here.
Comment 8 Dave Hyatt 2011-03-10 14:48:21 PST
Comment on attachment 85292 [details]
Patch

Can't use writing-mode like this.  That isn't what it is for at all.
Comment 9 Dave Hyatt 2011-03-10 14:49:38 PST
Meters orientation should just have to be explicitly specified IMO.  We need to just get that fixed in the standard.
Comment 10 Dimitri Glazkov (Google) 2011-03-10 14:52:15 PST
like <meter orientation="vertical">?
Comment 11 Dimitri Glazkov (Google) 2011-03-10 15:56:47 PST
(In reply to comment #9)
> Meters orientation should just have to be explicitly specified IMO.  We need to just get that fixed in the standard.

After talking with Hixie, he's suggesting adding "-webkit-direction" (or similar) CSS property instead. WDYT?
Comment 12 Ian 'Hixie' Hickson 2011-03-10 16:54:09 PST
The auto-orientation capability is similar to how Cocoa's NSSlider control automatically picks the best orientation (see its isVertical API). It increases the odds that people will make pretty pages.

I think -webkit-direction should have three values, 'auto' (the initial value, implementing the spec), and 'horizontal' and 'vertical' (which force a particular direction).

Maybe -webkit-control-direction to avoid confusion with 'direction'.
Comment 13 Hajime Morrita 2011-03-10 21:17:00 PST
Sorry for coming late and making the patch before reading simon's comment. (I just missed it...)

Okay, I'm going to introduce -webkit-control-direction.
The original intention to make direction explicit is that eliminate "auto"-like behavior
because it requires layout to determine direction, that causes complicated situation.
So I think it's OK to make the default value "horizontal" because 
there is no vertical meter indicator for native controls.
Comment 14 Dimitri Glazkov (Google) 2011-03-10 21:19:35 PST
(In reply to comment #13)
> Sorry for coming late and making the patch before reading simon's comment. (I just missed it...)
> 
> Okay, I'm going to introduce -webkit-control-direction.
> The original intention to make direction explicit is that eliminate "auto"-like behavior
> because it requires layout to determine direction, that causes complicated situation.
> So I think it's OK to make the default value "horizontal" because 
> there is no vertical meter indicator for native controls.

Hang on, hang on -- the discussion isn't over yet. Let Hyatt chime in.
Comment 15 Simon Fraser (smfr) 2011-03-11 09:04:24 PST
FYI I would prefer the name to contain 'orientation' rather than 'direction'. I'm still not sure that a normal property is the correct approach here. Maybe a pseudoclass?
Comment 16 Dimitri Glazkov (Google) 2011-03-11 09:10:25 PST
(In reply to comment #15)
> FYI I would prefer the name to contain 'orientation' rather than 'direction'. I'm still not sure that a normal property is the correct approach here. Maybe a pseudoclass?
Comment 17 Dimitri Glazkov (Google) 2011-03-11 09:12:10 PST
(In reply to comment #16)
> (In reply to comment #15)
> > FYI I would prefer the name to contain 'orientation' rather than 'direction'. I'm still not sure that a normal property is the correct approach here. Maybe a pseudoclass?

(In reply to comment #15)
> FYI I would prefer the name to contain 'orientation' rather than 'direction'. I'm still not sure that a normal property is the correct approach here. Maybe a pseudoclass?

Sorry for double-posting.  A pseudoclass wouldn't be enough, since it doesn't allow us to set the orientation by itself. Or perhaps I am misunderstanding... Can you pseudocode the pseudoclass idea for me?
Comment 18 Hajime Morrita 2011-03-25 01:20:26 PDT
How about to ask whatwg@ for dropping vertical meter from the spec?
There is no vertical meter/indicator on any platform and 
Opera, which is the only browser that supports <meter> other than WebKit, also doesn't support it.
Comment 19 Dimitri Glazkov (Google) 2011-03-25 07:39:08 PDT
(In reply to comment #18)
> How about to ask whatwg@ for dropping vertical meter from the spec?
> There is no vertical meter/indicator on any platform and 
> Opera, which is the only browser that supports <meter> other than WebKit, also doesn't support it.

Let's go with this notion for now. Implement horizontal only. We'll figure out what to do with spec later.
Comment 20 Hajime Morrita 2011-03-30 19:51:32 PDT
Created attachment 87660 [details]
Patch
Comment 21 Hajime Morrita 2011-03-30 19:52:44 PDT
Hi Dimitri, thank you for the suggestion!
> Let's go with this notion for now. Implement horizontal only. We'll figure out what to do with spec later.

So I removed vertical related code. It got simpler now.
Comment 22 Dimitri Glazkov (Google) 2011-03-30 20:15:22 PDT
Comment on attachment 87660 [details]
Patch

this should become even simpler once you switch it to the shadow DOM.
Comment 23 Hajime Morrita 2011-03-31 09:27:51 PDT
Created attachment 87746 [details]
Patch
Comment 24 Hajime Morrita 2011-03-31 09:29:06 PDT
Comment on attachment 87746 [details]
Patch

Oops. I intended to land it...
Comment 25 Hajime Morrita 2011-03-31 09:32:14 PDT
Committed r82589: <http://trac.webkit.org/changeset/82589>
Comment 26 WebKit Review Bot 2011-03-31 10:33:58 PDT
http://trac.webkit.org/changeset/82589 might have broken Qt Linux Release
The following tests are not passing:
fast/dom/HTMLMeterElement/meter-boundary-values.html
fast/dom/HTMLMeterElement/meter-optimums.html
fast/dom/HTMLMeterElement/meter-styles-changing-pseudo.html
fast/dom/HTMLMeterElement/meter-styles.html
Comment 27 Adam Barth 2011-03-31 11:15:44 PDT
I'm rolling out this patch because it turned many bots red and the author did not appear to be available to help repair them.
Comment 28 Hajime Morrita 2011-04-01 08:47:29 PDT
Created attachment 87863 [details]
Added skip list entries that need expectation update.
Comment 29 Dimitri Glazkov (Google) 2011-04-01 08:53:33 PDT
Comment on attachment 87863 [details]
Added skip list entries that need expectation update.

let's try again.
Comment 30 Hajime Morrita 2011-04-01 08:59:30 PDT
Committed r82686: <http://trac.webkit.org/changeset/82686>
Comment 31 WebKit Review Bot 2011-04-01 09:52:57 PDT
http://trac.webkit.org/changeset/82686 might have broken Windows 7 Release (Tests)
The following tests are not passing:
fast/dom/HTMLMeterElement/meter-appearances-capacity.html
fast/dom/HTMLMeterElement/meter-appearances-rating-relevancy.html