Closed Bug 659999 Opened 9 years ago Closed 8 years ago

Add a meterbar and meterchunk appearances

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: Vincent.Lamotte, Assigned: yoan.teboul)

References

(Blocks 2 open bugs)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file, 3 obsolete files)

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
Depends on: 555985
Version: unspecified → Trunk
Blocks: 555958
No longer depends on: 555985
Blocks: 555985
No longer blocks: 555958
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attached patch patch_v1.0 (obsolete) — Splinter Review
Added meterbar and meterchunk moz-appearances, and renamed a fonction in xpwidgets which can now be used for both meter and progress elements.
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 ;)
Attached patch patch_v1.1 (obsolete) — Splinter Review
Mounir's comment taken into account.
Attachment #536847 - Attachment is obsolete: true
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
Attachment #537125 - Flags: review?(roc)
Attachment #537125 - Flags: feedback+
Attached patch patch_v1.2 (obsolete) — Splinter Review
Attachment #537125 - Attachment is obsolete: true
Attachment #537125 - Flags: review?(roc)
Summary: Add a -moz-appearance: meterbar → Add a meterbar and meterchunk appearances
Duplicate of this bug: 660232
Assignee: nobody → yoan.teboul
Status: NEW → ASSIGNED
I don't know if the appearances are listed somewhere, in the doubt, adding 'dev-doc-needed'.
Keywords: dev-doc-needed
Attached patch patch_v1.3Splinter Review
Attachment #537142 - Attachment is obsolete: true
Blocks: 662870
Blocks: 663490
https://hg.mozilla.org/mozilla-central/rev/2a991342f5d5
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
You need to log in before you can comment on or make changes to this bug.