Last Comment Bug 659999 - Add a meterbar and meterchunk appearances
: Add a meterbar and meterchunk appearances
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Yoan TEBOUL
:
: Jet Villegas (:jet)
Mentors:
: 660232 (view as bug list)
Depends on:
Blocks: 555985 663490 662870
  Show dependency treegraph
 
Reported: 2011-05-26 10:10 PDT by Vincent LAMOTTE
Modified: 2012-12-11 08:18 PST (History)
8 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch_v1.0 (6.83 KB, patch)
2011-06-02 02:50 PDT, Yoan TEBOUL
no flags Details | Diff | Splinter Review
patch_v1.1 (4.26 KB, patch)
2011-06-03 05:19 PDT, Yoan TEBOUL
mounir: feedback+
Details | Diff | Splinter Review
patch_v1.2 (4.35 KB, patch)
2011-06-03 08:06 PDT, Yoan TEBOUL
roc: review+
Details | Diff | Splinter Review
patch_v1.3 (4.35 KB, patch)
2011-06-08 10:40 PDT, Yoan TEBOUL
no flags Details | Diff | Splinter Review

Description Vincent LAMOTTE 2011-05-26 10:10:14 PDT
User-Agent:       Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0.1) Gecko/20100101 Firefox/4.0.1
Build Identifier: 

Introduce a new meterbar choice for -moz-appearance.

Reproducible: Always
Comment 1 Yoan TEBOUL 2011-06-02 02:50:35 PDT
Created attachment 536847 [details] [diff] [review]
patch_v1.0

Added meterbar and meterchunk moz-appearances, and renamed a fonction in xpwidgets which can now be used for both meter and progress elements.
Comment 2 Mounir Lamouri (:mounir) 2011-06-02 04:03:06 PDT
Comment on attachment 536847 [details] [diff] [review]
patch_v1.0

You don't need the "-vertical" for meterbar and meterchunk.

>diff --git a/layout/style/nsCSSKeywordList.h b/layout/style/nsCSSKeywordList.h
>--- a/layout/style/nsCSSKeywordList.h
>+++ b/layout/style/nsCSSKeywordList.h
>@@ -328,16 +328,17 @@ CSS_KEY(lowercase, lowercase)
> CSS_KEY(ltr, ltr)
> CSS_KEY(manual, manual)
> CSS_KEY(margin-box, margin_box)
> CSS_KEY(matrix, matrix)
> CSS_KEY(medium, medium)
> CSS_KEY(menu, menu)
> CSS_KEY(menutext, menutext)
> CSS_KEY(message-box, message_box)
>+CSS_KEY(meter, meter)

Why is that needed?

>diff --git a/widget/src/xpwidgets/nsNativeTheme.cpp b/widget/src/xpwidgets/nsNativeTheme.cpp
>--- a/widget/src/xpwidgets/nsNativeTheme.cpp
>+++ b/widget/src/xpwidgets/nsNativeTheme.cpp
>@@ -467,18 +468,19 @@ nsNativeTheme::IsIndeterminateProgress(n
>     return aEventStates.HasState(NS_EVENT_STATE_INDETERMINATE);
>   }
>
>   return aFrame->GetContent()->AttrValueIs(kNameSpaceID_None, nsWidgetAtoms::mode,
>                                            NS_LITERAL_STRING("undetermined"),
>                                            eCaseMatters);
> }
>
>+// progressbar and meterbar:
> PRBool
>-nsNativeTheme::IsVerticalProgress(nsIFrame* aFrame)
>+nsNativeTheme::IsVerticalProgressMeter(nsIFrame* aFrame)

I would prefer to not change that for the moment. One reason is that the name is confusing because "progressmeter" is a XUL element. You could rename that "IsVerticalProgressOrMeter" to fix this but do we really need this new method for the moment? If you really want to add it, I think "IsVerticalFormControl" would be a better name given that other form controls might use the same system. And if you change the name, change the callers too ;)
Comment 3 Yoan TEBOUL 2011-06-03 05:19:42 PDT
Created attachment 537125 [details] [diff] [review]
patch_v1.1

Mounir's comment taken into account.
Comment 4 Mounir Lamouri (:mounir) 2011-06-03 06:32:03 PDT
Comment on attachment 537125 [details] [diff] [review]
patch_v1.1

>diff --git a/layout/style/forms.css b/layout/style/forms.css
> ::-moz-meter-bar {
>   display: inline-block ! important;
>   float: none ! important;
>   position: static ! important;
>   overflow: visible ! important;
>-  /* Add a new moz-appearance : Bug 660232
>-    * -moz-appearance: meterchunk;
>-    */
>+  -moz-appearance: meterchunk;

Could you put this on top of the block? and leave a blank line before "display: inline-block"?

f=me with that
Comment 5 Yoan TEBOUL 2011-06-03 08:06:34 PDT
Created attachment 537142 [details] [diff] [review]
patch_v1.2
Comment 6 Mounir Lamouri (:mounir) 2011-06-08 03:36:38 PDT
*** Bug 660232 has been marked as a duplicate of this bug. ***
Comment 7 Mounir Lamouri (:mounir) 2011-06-08 06:47:25 PDT
I don't know if the appearances are listed somewhere, in the doubt, adding 'dev-doc-needed'.
Comment 8 Yoan TEBOUL 2011-06-08 10:40:18 PDT
Created attachment 538052 [details] [diff] [review]
patch_v1.3
Comment 9 Mounir Lamouri (:mounir) 2012-06-06 06:21:21 PDT
https://hg.mozilla.org/mozilla-central/rev/2a991342f5d5

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