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

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Mounir Lamouri (:mounir)]

Attachment: Patch v1

Authors can't set the width of progress::-moz-progress-bar but should when the progress bar is in indeterminate state.

Comment 1

[Mounir Lamouri (:mounir)]

Attachment: Patch v1.1

I forgot to write tests.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Why should authors be able to set the width in this particular case?

Comment 3

[Mounir Lamouri (:mounir)]

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).

Comment 4

[Mounir Lamouri (:mounir)]

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?

Comment 5

[Mounir Lamouri (:mounir)]

CCing Boris in case of he wants to review it (the code part is only a few lines).

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

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.

Comment 7

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/78ea30b54e01

Comment 8

[Shawn Wilsher :sdwilsh]

Backed out in http://hg.mozilla.org/mozilla-central/rev/dd9ba28d2bd9 to resolve bug 655860.

Comment 9

[Mounir Lamouri (:mounir)]

The regression wasn't caused by these patches. Re-landed:
http://hg.mozilla.org/mozilla-central/rev/b5cdf8677461

Comment 10

[Eric Shepherd [:sheppy]]

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?

Comment 11

[Mounir Lamouri (:mounir)]

(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.

Comment 12

[Eric Shepherd [:sheppy]]

This is subtly mentioned in:

https://developer.mozilla.org/en/CSS/::-moz-progress-bar