226491 - Need to build a Progressmeter that supports undetermined mode

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Scott MacGregor]

On Windows, if you put the current progressmeter into undetermined mode, we
currently don't do anything. 

Ben and I were talking and it would be great if we had an XBL widget that was
used  instead of the existing progressmeter binding if the meter is in
undetermined mode.

In theory, one should be able to easily fake our own undetermined mode for the
progressmeter.

Comment 1

[Brian King [:kinger]]

Apparently this is affecting other platforms. There is a recent thread on
n.p.m.xpfe about it entitled 'No "undetermined" progressbar on Linux'.

Comment 2

[Scott MacGregor]

I'm not actively working on this if someone on that thread wants to work on it.
I think it requires changes to the native theme apis and probably some xbl magic
too.

Comment 3

[[e-mail]]

Attachment: Possible patch

I don't know why the background image doesn't animate, so I didn't CC myself.

Comment 4

[Laurent Jouanneau]

it seems that the image is not in any jar files... (firefox 0.9.1)

so, the image progressmeter-busy.gif must be added in the jar file : 
classix.jar:skin/global/progressmeter/

th patch must be added, and then it works.

(sorry, i can't correct it directly, no access to cvs ...)

Comment 5

[Scott MacGregor]

That's not really the problem. For the classic theme our progressmeter is a
native looking progressmeter.

We need to write some XBL to produce a native looking progressmeter in
undetermined mode.

We can leverage parts of the existing marquee tag and the stack tag to animate a
progress chunk back and forth over a progressbar.

Test mock up coming up.

Would be awesome if we could get someone to write an XBL widget to do this. 

Comment 6

[Scott MacGregor]

Attachment: screen shot using a marquee tag and a stack

Comment 7

[Scott MacGregor]

Attachment: here is a demo

Comment 8

[[e-mail]]

Attachment: Hacky patch

I ran into a couple of issues:

the size of the progressmeter isn't known immediately, so I just set the size
of the scrolling bitty in the loop, although I should only need to set it once

the stack doesn't mind moving things off its left but objects if you try to
move things off its right, so I use a large negative margin to work around it.

Comment 9

[Scott MacGregor]

Attachment: screen shot of neil's patch

looks pretty cool

Comment 10

[Scott MacGregor]

Comment on attachment 153574 [details] [diff] [review]
Hacky patch

Would it make more sense to put the XBL binding for the progressmeter over in
xul.css (where the content binding for progressmeter exists) instead of
global.css?

Comment 11

[Scott MacGregor]

I did see one thing. When the progressmeter leaves the undetermined state (or
gets hidden because there is no more progress to report), the timeout is still
firing leading to the following exception:

JavaScript error: chrome://global/content/bindings/progressmeter.xml, line 67: u
ncaught exception: [Exception... "Component returned failure code: 0x80004005 (N
S_ERROR_FAILURE) [nsIDOMXULElement.boxObject]"  nsresult: "0x80004005 (NS_ERROR_
FAILURE)"  location: "JS frame :: chrome://global/content/bindings/progressmeter
.xml :: nextStep :: line 67"  data: no]

Comment 12

[Scott MacGregor]

actually that exception is probably because of the way I happen to be using the
widget. I bet I'm hiding it without taking it out of the undetermined state so
the destructor never gets called. Probably something the consumer should have to
worry about and not necessarily the widget itself.

Comment 13

[[e-mail]]

Attachment: Less unreliable patch

The reason I put the style rule in Classic's global.css is that Modern (and
possibly third party themes) already has an undetermined progressmeter.

Comment 14

[Scott MacGregor]

Neil this "less unreliable patch" is working really well for me so far.

Comment 15

[Scott MacGregor]

I'm ready to run with this patch if you are Neil. I've been using it all day and
it's working great. Anything else you want to do it before we get some code
reviews going?

Comment 16

[[e-mail]]

It occurs to me that the old background image won't be necessary.

Comment 17

[Scott MacGregor]

Attachment: aviary version of Neil's patch

Comment 18

[Malcolm Rowe]

Comment on attachment 153946 [details] [diff] [review]
Less unreliable patch

Apologies if these are stupid questions, but..

We're not changing the Mac progressmeter?
http://lxr.mozilla.org/mozilla/source/themes/classic/global/mac/progressmeter.c
ss#58
Or the tree versions?
http://lxr.mozilla.org/mozilla/source/themes/classic/global/mac/tree.css#187
http://lxr.mozilla.org/mozilla/source/themes/classic/global/win/tree.css#205

Comment 19

[[e-mail]]

Ah, ok, so trees still use that image. And until trees can draw native themed
progressmeters, then no, we're not changing the tree versions. Also my
understanding was that the Mac has a native undetermined progressmeter.

Comment 20

[Scott MacGregor]

I'm getting some reports of folks seeing the progress chunk start outside the
boundary of the progressmeter. Here's a screen shot:

http://pub.tsn.dk/screendumps/thunderbird.bug2.jpg

This looks like it was from win2k. I'll test that out later today.

Comment 21

[Jude Cooks]

(In reply to comment #20)
> I'm getting some reports of folks seeing the progress chunk start outside the
> boundary of the progressmeter. Here's a screen shot:
> 
> http://pub.tsn.dk/screendumps/thunderbird.bug2.jpg
> 
> This looks like it was from win2k. I'll test that out later today.

I also see this with build 20040726 using WinXP.  Same as the screenshot.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #20)
> I'm getting some reports of folks seeing the progress chunk start outside the
> boundary of the progressmeter. Here's a screen shot:
> 
> This looks like it was from win2k. I'll test that out later today.

Same for me. I missed this bug so i reported this earlier today in bug 253232.

Comment 23

[[e-mail]]

OK, there are two ways to fix bug 253232:

Either add stack.progress-remainder { overflow: -moz-hidden-unscrollable; } to
progressmeter.css

Or add style="overflow: -moz-hidden-unscrollable;" to progressmeter.xul

Comment 24

[Doug Wright]

This works for me - but seems *VERY* fast. Can it be slowed down a little?

Comment 25

[jag (Peter Annema)]

Comment on attachment 153946 [details] [diff] [review]
Less unreliable patch

sr=jag

Comment 26

[[e-mail]]

(In reply to comment #24)
>This works for me - but seems *VERY* fast. Can it be slowed down a little?
mscott, do you think we should slow it down a bit?

Comment 27

[Scott MacGregor]

that's a tricky question Neil. I notice a big difference between the cycling
speed when the chunk is small (such as the main mail 3pane window) vs when the
chunk is large (such as the mail compose progress dialog. A value good for one
looks too fast or to slow for the other.

That being said, I did some experimentation and found a value of 20 instead of
10 for the timer seemed better for both scenarios....just my 2 cents

Comment 28

[[e-mail]]

Attachment: Slower version

Comment 29

[[e-mail]]

Comment on attachment 155259 [details] [diff] [review]
Slower version

Hopefully this version runs at a reasonable speed on the send progress dialog.

Comment 30

[Scott MacGregor]

Comment on attachment 155259 [details] [diff] [review]
Slower version

The send progress dialog now works perfect. Maybe its my imagination but now
the main mail 3 pane seems too fast. Maybe this patch + tweaking the timer
interval will give us the ideal result. I'll play around with it.

Comment 31

[Scott MacGregor]

Comment on attachment 155259 [details] [diff] [review]
Slower version

Change the >> 2 to >> 7 like you sent me over e-mail

and r/sr=mscott

Comment 32

[[e-mail]]

Attachment: Final version

I couldn't stop twiddling until I got to this patch which now feels right :-)

Comment 33

[Scott MacGregor]

Comment on attachment 155349 [details] [diff] [review]
Final version

that seems to work just right!

Comment 34

[[e-mail]]

Final version checked in to trunk.

Comment 35

[Peter van der Woude [:Peter6]]

Since there is a fix, requesting
?1.0PR

Comment 36

[Scott MacGregor]

this is already in the aviary branch. No need to request something that's alrady
there.

Comment 37

[Ian Neal]

Scott,
The changes to the 3 css files did not make it onto the aviary branch, though
the changes to the xul file did I think.

Comment 38

[Scott MacGregor]

this is working fine on the aviary branch. 

Comment 39

[Ian Neal]

http://neil.rashbrook.org/pyeximon.xul
Works fine on nightly but not on 0.10.1 so something hasn't made it into the branch

Comment 40

[Ian Neal]

Logged bug 266459 on problem

Comment 41

[Gary van der Merwe]

As pointed out by Ian, the changes to the files
mozilla/themes/classic/global/win/global.css
mozilla/themes/classic/global/unix/global.css
has not made it to avery trunk.

I got my avery trunk from
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ .

Also according to
http://bonsai.mozilla.org/cvsgraph.cgi?file=mozilla/themes/classic/global/win/global.css
we can see that version 1.43 of this file (the version that contains the patch
for this bug) is not in any avery / firefox branch. 

(May I reopen this bug?)

Comment 42

[[e-mail]]

Aviary doesn't use the mozilla/themes folders.

Comment 43

[Gary van der Merwe]

Where dose skin\classic\global\global.css in chrome\classic.jar come from in the
cvs?

Comment 44

[Gary van der Merwe]

I've workout what's going on here. I've submited patch to Bug 266459