Cocoa's progress bar widget should use HTML progress element's values

RESOLVED FIXED in mozilla6

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mounir, Assigned: mounir)

Tracking

Trunk
mozilla6
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Created attachment 512754 [details] [diff] [review]
Patch v1

Cocoa's progress bar widget is the only widget that actually draw itself the bar so it needs the current value and the max value. For the moment, the code doing that is assuming that the element is a XUL progressmeter element.

This patch makes it work with an HTML progress element. There is a [dirty] hack that consist of multiplying the float value we get because Cocoa's widget takes only integers for value and max.
Attachment #512754 - Flags: review?(mstange)
(Assignee)

Updated

6 years ago
Attachment #512754 - Flags: review?(roc)
(Assignee)

Updated

6 years ago
Whiteboard: [needs review]
(Assignee)

Updated

6 years ago
Depends on: 514437
How about this: Make GetProgressValue and GetProgressMaxValue return a float, and then in DrawProgress always set tdi.max to PR_INT32_MAX and do some calculations to get the correct value.
Then we always use the best precision we can get and don't have to choose an arbitrary number.
(Assignee)

Comment 2

6 years ago
Created attachment 512790 [details] [diff] [review]
Patch v1.1

Should be better now even if I spent half an hour before understanding why PR_INT32_MAX*1 gives PR_INT32_MIN ;)
Attachment #512754 - Attachment is obsolete: true
Attachment #512790 - Flags: review?(mstange)
Attachment #512754 - Flags: review?(roc)
Attachment #512754 - Flags: review?(mstange)
(Assignee)

Updated

6 years ago
Attachment #512790 - Flags: review?(roc)
Comment on attachment 512790 [details] [diff] [review]
Patch v1.1

Oh no :(

Maybe roc has a better idea how to make this presentable.
Attachment #512790 - Flags: review?(mstange) → review+
(Assignee)

Comment 4

6 years ago
Created attachment 515604 [details] [diff] [review]
Patch v1.2

r=mstange

The HTML progress element is now using doubles which is fixing the rounding issue. Markus, do you still want Roc's review?
Attachment #512790 - Attachment is obsolete: true
Attachment #515604 - Flags: review?(roc)
Attachment #512790 - Flags: review?(roc)
Attachment #515604 - Flags: review?(roc) → review+
(Assignee)

Updated

6 years ago
Whiteboard: [needs review] → [ready to land][waits for dependencies]
(Assignee)

Updated

6 years ago
Blocks: 634551
(Assignee)

Comment 5

6 years ago
Pushed:
http://hg.mozilla.org/mozilla-central/rev/94dc6447f102
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [ready to land][waits for dependencies]
Target Milestone: --- → mozilla6

Updated

6 years ago
Depends on: 655860
Backed out in http://hg.mozilla.org/mozilla-central/rev/dd9ba28d2bd9 to resolve bug 655860.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 7

6 years ago
The regression wasn't caused by these patches. Re-landed:
http://hg.mozilla.org/mozilla-central/rev/5b69bdf44737
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
No longer depends on: 655860
You need to log in before you can comment on or make changes to this bug.