Something like: progress::-moz-progress-bar

For reference: https://svn.webkit.org/wiki/Styling%20Form%20Controls

(In reply to comment #1) > For reference: https://svn.webkit.org/wiki/Styling%20Form%20Controls FWIW, I don't like the name they chose.

I haven't been able to do anything else than setting the border and the background-color with webkit's pseudo-element. I think we should let the authors manipulate the bar like any element.

Actually, it's not really WIP but I didn't test the patch against the test suite I wrote (not on the same computer) but it should be fine.

Actually, the tests cases showed an edge-case issue. Looks like they are useful ;)

I-just-realized-I-forgot-something-when-I-pressed-Submit syndrome :(

Comment on attachment 512283 [details] [diff] [review] Patch v1 I think you need to do something to ensure that the pseudo-element style context is set up correctly when we restyle the frame ... but I'm not sure what.

Comment on attachment 512283 [details] [diff] [review] Patch v1 Given what you do in nsProgressFrame::SetInitialChildList, you need to enforce a few invariants that would have caused us to construct a different sort of frame. In particular, forms.css needs to have: ::-moz-progress-bar { /* block styles that would change the type of frame we construct */ display: block ! important; float: none ! important; position: static ! important; overflow: visible ! important; } And while I'm on the topic, I don't see any reason the selector shouldn't change to what I wrote above (rather than progress::-moz-progress-bar. That said, rather than overriding SetInitialChildList the way you do, I think you should implement nsIAnonymousContentCreator::CreateFrameFor to create the frame with the correct style context initially. > // We want the frame to take all the available size. >- reflowState.SetComputedWidth(availSize.width); >+ nscoord width = availSize.width - reflowState.mComputedMargin.LeftRight() >+ - reflowState.mComputedBorderPadding.LeftRight(); >+ width = NS_MAX(width, 0); >+ reflowState.SetComputedWidth(width); Instead of doing this, why not just stick width: auto ! important in forms.css ? It seems like that's basically what you're doing here. But not using the same nsHTMLReflowMetrics for two different frames certainly looks good. That's a problem introduced in bug 567872's patch, though. >+ if (GetStyleVisibility()->mDirection == NS_STYLE_DIRECTION_RTL) { >+ xoffset += reflowState.mComputedMargin.right; >+ } else { >+ xoffset += reflowState.mComputedMargin.left; > } This is wrong; you should just add the left margin, since you're determining the top-left corner of the frame. I believe restyling the frame should just work. >+ newStyleContext = barFrame->PresContext()->PresShell()->StyleSet()-> At least omit the "->PresShell()", since PresContext() has a StyleSet(). Sorry for the huge delay getting to this (and bug 567872). I should be able to look at a revised patch much faster.

(In reply to comment #8) > > // We want the frame to take all the available size. > >- reflowState.SetComputedWidth(availSize.width); > >+ nscoord width = availSize.width - reflowState.mComputedMargin.LeftRight() > >+ - reflowState.mComputedBorderPadding.LeftRight(); > >+ width = NS_MAX(width, 0); > >+ reflowState.SetComputedWidth(width); > > Instead of doing this, why not just stick width: auto ! important in > forms.css ? It seems like that's basically what you're doing here. Er, ignore this comment, mostly. Except I think you actually should be passing something different to the reflow state constructor, and only using this calculation for the SetComputedWidth call. But I'll deal with that in bug 567872.

(In reply to comment #8) > That said, rather than overriding SetInitialChildList the way you do, I > think you should implement nsIAnonymousContentCreator::CreateFrameFor to > create the frame with the correct style context initially. If I understand correctly, I will have to define a nsCSSFrameConstructor::ConstructProgressFrame method if I use CreateFrameFor. Does it worth adding this complexity?

You shouldn't need a separate ctor method. Just need to make sure your frame is an nsIAnonymousContentCreator and not a leaf. Or is it a leaf?

(In reply to comment #11) > You shouldn't need a separate ctor method. Just need to make sure your frame > is an nsIAnonymousContentCreator and not a leaf. Or is it a leaf? It's a nsIAnonymousContentCreator and it's not a leaf AFAIUI (it has a child). Though, I get these asserts: ###!!! ASSERTION: If you need to use CreateFrameFor, you need to call CreateAnonymousFrames manually and not follow the standard ProcessChildren() codepath for this frame: '!creator || !creator->CreateFrameFor(anonymousItems[i])', file /home/volkmar/projects/mozilla/html5forms/layout/base/nsCSSFrameConstructor.cpp, line 9546 ###!!! ASSERTION: asked to create frame construction item for a node that already has a frame: 'Error', file /home/volkmar/projects/mozilla/html5forms/layout/base/nsCSSFrameConstructor.cpp, line 4998 There seem to be two consumers of CreateFrameFor and both have a ctor method in nsCSSFrameConstructor calling CreateAnonymousFrames.

Oh, you need to use CreateFrameFor? In that case, yeah, that would involve having a ctor function... But you don't really want to create your own frame, right? Just to hand out a style context? We've talked about changing nsIAnonymousContentCreator to hand out style contexts, not just content nodes, before...

(In reply to comment #13) > Oh, you need to use CreateFrameFor? In that case, yeah, that would involve > having a ctor function... I don't know if I *need*. David has recommended that approach on comment 8. > But you don't really want to create your own frame, right? Just to hand out a > style context? We've talked about changing nsIAnonymousContentCreator to hand > out style contexts, not just content nodes, before... I guess so, yes.

Fwiw, I would be happy to write a patch to that effect if you want.

Add RTL tests and change display: block !important to display: inline-block !important (tests were failing).

(In reply to comment #15) > Fwiw, I would be happy to write a patch to that effect if you want. I've open bug 654989.

Comment on attachment 530284 [details] [diff] [review] Patch v1.2 r=dbaron, but please file a followup on fixing this SetInitialChildList stuff to use the api added in bug 654989.

(In reply to comment #20) > Comment on attachment 530284 [details] [diff] [review] [review] > Patch v1.2 > > r=dbaron, but please file a followup on fixing this SetInitialChildList > stuff to use the api added in bug 654989. I've open bug 654990 for that.

Pushed: http://hg.mozilla.org/mozilla-central/rev/2139b047dbb7

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

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

Is the point of this to be able to access the currently-filled part of the progress bar within the full progress bar?

(In reply to comment #25) > Is the point of this to be able to access the currently-filled part of the > progress bar within the full progress bar? Yes, it will let the author style the bar inside the progress bar.

Documented: https://developer.mozilla.org/en/CSS/::-moz-progress-bar Also updated: https://developer.mozilla.org/en/HTML/Element/progress and Firefox 6 for developers.