As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 656284 - XUL progressmeter is clipped on Windows
: XUL progressmeter is clipped on Windows
Status: VERIFIED FIXED
: regression
Product: Core
Classification: Components
Component: Widget: Win32 (show other bugs)
: Trunk
: x86 Windows 7
: P1 normal with 3 votes (vote)
: mozilla6
Assigned To: Mounir Lamouri (:mounir)
:
: Jim Mathies [:jimm]
Mentors:
: 656592 656842 (view as bug list)
Depends on:
Blocks: 568825
  Show dependency treegraph
 
Reported: 2011-05-11 07:01 PDT by Jim Mathies [:jimm]
Modified: 2011-12-26 01:47 PST (History)
19 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
download progress (101.67 KB, image/png)
2011-05-11 07:01 PDT, Jim Mathies [:jimm]
no flags Details
Patch v1 (3.93 KB, patch)
2011-05-13 03:21 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
With -moz-box-sizing (3.45 KB, patch)
2011-05-14 10:53 PDT, Mounir Lamouri (:mounir)
roc: review+
Details | Diff | Splinter Review
screenshot (46.59 KB, image/jpeg)
2011-05-15 18:26 PDT, pal-moz
no flags Details
With tests update (4.85 KB, patch)
2011-05-16 03:18 PDT, Mounir Lamouri (:mounir)
no flags Details | Diff | Splinter Review
Download Window (5-17-11) (83.09 KB, image/png)
2011-05-17 06:53 PDT, Chris B.
no flags Details
Download Window (5-17-11) (181.58 KB, image/png)
2011-05-17 06:55 PDT, Chris B.
no flags Details
Screenshot after 2nd patch (2.20 KB, image/png)
2011-07-20 04:45 PDT, George Carstoiu
no flags Details

Description User image Jim Mathies [:jimm] 2011-05-11 07:01:31 PDT
Created attachment 531620 [details]
download progress

STR:

1) open download window
2) download something

see screenshot.

Looks like we are going to have to track down that weird lower bounds. (kProgressDeterminedXPOverflow is the patch in bug 568825)
Comment 1 User image Mounir Lamouri (:mounir) 2011-05-11 07:14:39 PDT
This is really weird because, indeed, there is an issue with the download window but also, on my Windows VM, I don't have any overflow now (that means all progress bars look like the download window progress bars) but I do on Nightly.
Comment 2 User image Chris B. 2011-05-11 08:10:08 PDT
This also occurs in the very quick pop-up for the "Processing Print" submission dialog.
Comment 3 User image Mounir Lamouri (:mounir) 2011-05-11 08:18:31 PDT
Actually, it seems to happen with all XUL progressmeter element.
Comment 4 User image Raul Malea 2011-05-11 13:15:13 PDT
Also in progress add-ons verify after change Firefox 4 to Nightly.
Comment 5 User image Mounir Lamouri (:mounir) 2011-05-12 06:27:01 PDT
*** Bug 656592 has been marked as a duplicate of this bug. ***
Comment 6 User image John P Baker 2011-05-13 02:43:42 PDT
*** Bug 656842 has been marked as a duplicate of this bug. ***
Comment 7 User image Mounir Lamouri (:mounir) 2011-05-13 03:21:52 PDT
Created attachment 532156 [details] [diff] [review]
Patch v1

Roc, this patch seems to fix the issue though I still don't understand why the issue is only showing on Windows.
Comment 8 User image Mounir Lamouri (:mounir) 2011-05-13 03:23:33 PDT
We really need this to be fixed for Firefox 6 branching. Otherwise, <progress> will probably miss the Firefox 6 train :(
Comment 9 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-13 04:04:07 PDT
Can you explain why you're making these changes? I don't understand what the bug is or how these changes fix it.
Comment 10 User image Mounir Lamouri (:mounir) 2011-05-13 04:14:50 PDT
(In reply to comment #9)
> Can you explain why you're making these changes? I don't understand what the
> bug is or how these changes fix it.

It's a regression from bug 568825. I have to admit I don't really understand what is happening but basically, there was this overflow that I thought was a bug somewhere between our Windows widget and Windows. So I wrote a dirty workaround that happens to broke XUL progressmeter.
It seems patch fixes the issue for HTML progress elements and doesn't regress XUL progressmeter. I'm not sure if that's a real fix but it seems to work and is obviously better than the previous fix.
I believe you have better knowledge of layout/ and widget/ code that me so you might understand better than me what's happening...
Comment 11 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-14 04:16:13 PDT
Comment on attachment 532156 [details] [diff] [review]
Patch v1

Review of attachment 532156 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/forms/nsProgressFrame.cpp
@@ +173,5 @@
>  {
>    bool vertical = GetStyleDisplay()->mOrient == NS_STYLE_ORIENT_VERTICAL;
>    nsHTMLReflowState reflowState(aPresContext, aReflowState, aBarFrame,
>                                  nsSize(aReflowState.ComputedWidth(),
> +                                       aReflowState.ComputedHeight()));

Setting an available height seems wrong for the reasons we discussed in the other bug.
Comment 12 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-14 04:17:35 PDT
I don't understand what these overflow values are supposed to mean.
Comment 13 User image Mounir Lamouri (:mounir) 2011-05-14 10:53:50 PDT
Created attachment 532450 [details] [diff] [review]
With -moz-box-sizing

While looking around the code I realized that "-moz-box-sizing: border-box;" was used to prevent some overflows. I don't know if not using that was a real mistake but it seems to fix the issue for vertical and horizontal progress bars on Windows and do not regress anything on GTK.

Roc, does that sound like a real fix or still a workaround? If the later, is it safe?
Comment 14 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-15 16:33:32 PDT
Comment on attachment 532450 [details] [diff] [review]
With -moz-box-sizing

If this works, great.

Can you make a reftest?
Comment 15 User image pal-moz 2011-05-15 18:26:25 PDT
Created attachment 532549 [details]
screenshot

on Windows XP, meter is blank/empty.


GPU Accelerated Windows : 0/1
Comment 16 User image Mounir Lamouri (:mounir) 2011-05-16 03:18:39 PDT
Created attachment 532591 [details] [diff] [review]
With tests update

I had a reftest checking that there were no overflow. I guess I can keep it. To test the XUL regression, the only idea I have is to compare a very small <progresmeter> with a <progressmeter> styled so there is no bar (like done for <html:progres> in layout/reftests/native-theme/). Though, I don't know how to do that in XUL (can I access an anonymous element from a reftest?).

Please, let me know if you see a way to test that.
Comment 17 User image Mounir Lamouri (:mounir) 2011-05-16 06:26:04 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/93656440018a
Comment 18 User image Mounir Lamouri (:mounir) 2011-05-17 06:04:18 PDT
Can anyone confirm that the bug is fixed in the current Nightly and mark the bug as VERIFIED?
Comment 19 User image Chris B. 2011-05-17 06:13:36 PDT
(In reply to comment #18)
> Can anyone confirm that the bug is fixed in the current Nightly and mark the
> bug as VERIFIED?

Nightly isn't out yet - but I will confirm when the build is available to download.
Comment 20 User image Chris B. 2011-05-17 06:49:13 PDT
PARTIALLY confirmed.  There is still some extra white space on the bottom of the progress bar about a couple pixels wide, but it still looks better than the original screenshot above "download progress".  Still needs a slight modification, though.....
Comment 21 User image Chris B. 2011-05-17 06:53:12 PDT
Created attachment 532950 [details]
Download Window (5-17-11)
Comment 22 User image Chris B. 2011-05-17 06:54:06 PDT
(In reply to comment #21)
> Created attachment 532950 [details]
> Download Window (5-17-11)

Sorry....wrong upload.....
Comment 23 User image Chris B. 2011-05-17 06:55:03 PDT
Created attachment 532951 [details]
Download Window (5-17-11)
Comment 24 User image Josh Tumath 2011-05-17 08:00:41 PDT
The gap appears all the way around the bar, as though it has 1px padding. It's just more noticeable at the bottom.
Comment 25 User image Chris B. 2011-05-17 08:22:51 PDT
(In reply to comment #24)
> The gap appears all the way around the bar, as though it has 1px padding.
> It's just more noticeable at the bottom.

Right - but I believe if the progress bar is say 20 pixels high, the entire 20 pixels should be filled in, like with the 5/10/11 build (last working one) and not only 14 or 16 pixels centered throughout.
Comment 26 User image George Carstoiu 2011-07-20 04:45:12 PDT
Created attachment 547034 [details]
Screenshot after 2nd patch

Mozilla/5.0 (Windows NT 6.1; rv:8.0a1) Gecko/20110719 Firefox/8.0a1

Verified issue on the latest trunk. Adding a screenshot of how the download progress bar looks in the latest Nightly.
Comment 27 User image George Carstoiu 2011-07-20 04:50:52 PDT
Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0

Verified issue also on 6.0b2 - not reproducible.
Comment 28 User image Wesley Johnston (:wesj) 2011-07-20 07:39:28 PDT
How was this verified fixed if there are still issues here?
Comment 29 User image George Carstoiu 2011-07-20 07:47:47 PDT
Looking at the latest trunk I haven't noticed any issues. Can you please elaborate on what issues are you still observing?

Thanks!
Comment 30 User image Chris B. 2011-07-20 08:22:42 PDT
(In reply to comment #29)
> Looking at the latest trunk I haven't noticed any issues. Can you please
> elaborate on what issues are you still observing?
> 
> Thanks!

As stated in comment 20 two months ago, the green fill in does NOT fill the entire rectangle when a file is downloading.  This also appears during the "Print Preparation" dialog.  I still see this issue in the latest Nightly.  Please refer to the attachments above:
  1) Download Window (5-17-11)
  2) Screenshot after 2nd patch
Comment 31 User image Philip Chee 2011-07-20 08:51:00 PDT
Please provide a new screenshot using the latest nightly Firefox 8.0a1
Comment 32 User image Peter Henkel [:Terepin] 2011-07-20 08:53:04 PDT
This bug is there in the same form from the day this bug landed until today. What's the point of posting another screenshot showing the same thing? You think we are making this up just to make you mad or something?
Comment 33 User image Mounir Lamouri (:mounir) 2011-07-20 10:27:22 PDT
This bug is fixed, the remaining problem will be treated in bug 658885.

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