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

RESOLVED FIXED in mozilla6

Status

()

defect
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

({dev-doc-complete})

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Posted patch Patch v1 (obsolete) — Splinter Review
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)
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
Posted patch Patch v1.1Splinter Review
I forgot to write tests.
Attachment #520095 - Attachment is obsolete: true
Attachment #520095 - Flags: review?(dbaron)
Attachment #520101 - Flags: review?(dbaron)
Depends on: 638176
Why should authors be able to set the width in this particular case?
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).
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?
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+
Whiteboard: [needs review] → [ready to land][waits for dependencies]
Pushed:
http://hg.mozilla.org/mozilla-central/rev/78ea30b54e01
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [ready to land][waits for dependencies]
Target Milestone: --- → mozilla6
Depends on: 655860
Backed out in http://hg.mozilla.org/mozilla-central/rev/dd9ba28d2bd9 to resolve bug 655860.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
The regression wasn't caused by these patches. Re-landed:
http://hg.mozilla.org/mozilla-central/rev/b5cdf8677461
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
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?
(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.
You need to log in before you can comment on or make changes to this bug.