Let authors set the progress bar's width when in indeterminate state

RESOLVED FIXED in mozilla6

Status

()

Core
Layout: Form Controls
RESOLVED FIXED
7 years ago
6 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

({dev-doc-complete})

Trunk
mozilla6
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
Created attachment 520095 [details] [diff] [review]
Patch v1

Authors can't set the width of progress::-moz-progress-bar but should when the progress bar is in indeterminate state.
Attachment #520095 - Flags: review?(dbaron)
(Assignee)

Updated

7 years ago
Summary: Let authors to set the progress bar's width when in indeterminate state → Let authors set the progress bar's width when in indeterminate state
(Assignee)

Comment 1

7 years ago
Created attachment 520101 [details] [diff] [review]
Patch v1.1

I forgot to write tests.
Attachment #520095 - Attachment is obsolete: true
Attachment #520095 - Flags: review?(dbaron)
Attachment #520101 - Flags: review?(dbaron)
(Assignee)

Updated

7 years ago
Depends on: 638176
Why should authors be able to set the width in this particular case?
(Assignee)

Comment 3

6 years ago
The answer is in the reason why I chose to forbid width change in the other case: if <progress> is determinate, the bar width is known and we want to prevent the author to change it. Otherwise, there is no reason why the author should be forbidden to change it and we can imagine that some authors might want to animate the bar a la GTK (width=20% bouncing on each side) and some others might want to do it a la MacOS or Windows (width=100% with an internal animation).
(Assignee)

Comment 4

6 years ago
We are close to have <progress> ready to be pushed (and thus being in the FF6 train). Do you think you can review that patch soon?
(Assignee)

Comment 5

6 years ago
CCing Boris in case of he wants to review it (the code part is only a few lines).
Comment on attachment 520101 [details] [diff] [review]
Patch v1.1

>-  if (position >= 0.0f) {
>+  if (position >= 0.0) {

Could you put this in progress-layout.patch where it belongs?  (It's relatively easy to do by just editing the patches and then qref-ing the one that, as a result, ends up with the + and - lines the same.  Just qcommit first in case you mess up...)

r=dbaron, though I think I made some comments on the code that you're indenting when reviewing one of the patches underneath.
Attachment #520101 - Flags: review?(dbaron) → review+
(Assignee)

Updated

6 years ago
Whiteboard: [needs review] → [ready to land][waits for dependencies]
(Assignee)

Comment 7

6 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/78ea30b54e01
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [ready to land][waits for dependencies]
Target Milestone: --- → mozilla6

Updated

6 years ago
Depends on: 655860
Backed out in http://hg.mozilla.org/mozilla-central/rev/dd9ba28d2bd9 to resolve bug 655860.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 9

6 years ago
The regression wasn't caused by these patches. Re-landed:
http://hg.mozilla.org/mozilla-central/rev/b5cdf8677461
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
No longer depends on: 655860
I'm undecided on whether or not this really needs specific documentation, given that it's a behavior change to a behavior that was never written about in the first place. It seems to me that the new behavior is what people would expect in the first place.

Is there any specific reason this needs to be called out expressly?
(Assignee)

Comment 11

6 years ago
(In reply to comment #10)
> I'm undecided on whether or not this really needs specific documentation,
> given that it's a behavior change to a behavior that was never written about
> in the first place. It seems to me that the new behavior is what people
> would expect in the first place.
> 
> Is there any specific reason this needs to be called out expressly?

I thought that mentioning that in the :-moz-progress-bar doc would have been good. Though, if you don't think so, I'm fine with that.
This is subtly mentioned in:

https://developer.mozilla.org/en/CSS/::-moz-progress-bar
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.