Last Comment Bug 642667 - Let authors set the progress bar's width when in indeterminate state
: Let authors set the progress bar's width when in indeterminate state
Status: RESOLVED FIXED
: dev-doc-complete
Product: Core
Classification: Components
Component: Layout: Form Controls (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Mounir Lamouri (:mounir)
:
Mentors:
Depends on: 638176
Blocks: 633207
  Show dependency treegraph
 
Reported: 2011-03-17 17:43 PDT by Mounir Lamouri (:mounir)
Modified: 2011-06-16 14:01 PDT (History)
5 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (3.02 KB, patch)
2011-03-17 17:43 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Patch v1.1 (6.87 KB, patch)
2011-03-17 17:59 PDT, Mounir Lamouri (:mounir)
dbaron: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2011-03-17 17:43:40 PDT
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.
Comment 1 Mounir Lamouri (:mounir) 2011-03-17 17:59:14 PDT
Created attachment 520101 [details] [diff] [review]
Patch v1.1

I forgot to write tests.
Comment 2 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-04-29 14:20:24 PDT
Why should authors be able to set the width in this particular case?
Comment 3 Mounir Lamouri (:mounir) 2011-05-02 15:46:21 PDT
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) 2011-05-04 09:21:18 PDT
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) 2011-05-05 05:52:00 PDT
CCing Boris in case of he wants to review it (the code part is only a few lines).
Comment 6 David Baron :dbaron: ⌚️UTC+2 (review requests must explain patch) 2011-05-05 12:16:17 PDT
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) 2011-05-09 05:37:37 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/78ea30b54e01
Comment 8 Shawn Wilsher :sdwilsh 2011-05-09 16:12:37 PDT
Backed out in http://hg.mozilla.org/mozilla-central/rev/dd9ba28d2bd9 to resolve bug 655860.
Comment 9 Mounir Lamouri (:mounir) 2011-05-10 06:57:53 PDT
The regression wasn't caused by these patches. Re-landed:
http://hg.mozilla.org/mozilla-central/rev/b5cdf8677461
Comment 10 Eric Shepherd [:sheppy] 2011-05-31 11:58:01 PDT
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) 2011-06-01 02:30:03 PDT
(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] 2011-06-16 14:01:03 PDT
This is subtly mentioned in:

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

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