"Applying Update..." spinner is hard to see in Developer Edition

VERIFIED FIXED in Firefox 44

Status

()

Firefox
Theme
VERIFIED FIXED
3 years ago
2 years ago

People

(Reporter: bgrins, Assigned: bgrins)

Tracking

40 Branch
Firefox 44
Points:
---

Firefox Tracking Flags

(firefox44 verified)

Details

(Whiteboard: [devedition-polish][polish-backlog][difficulty=easy][bugday-20160113], URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(7 attachments)

(Assignee)

Description

3 years ago
Created attachment 8620702 [details]
dark-spinner.png

The spinner in 'About Firefox' that shows up when an update is applying is dark against the dark dev edition background.  See screenshot.
(Assignee)

Updated

3 years ago
Whiteboard: [devedition-polish]
PDFjs has an inverted spinner that we could use: http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/content/web/images/loading-small.png
This also applies to Nightly.
Maybe it would be nice to update them here (bug 750054)
(Assignee)

Comment 4

3 years ago
(In reply to Guillaume C. [:ge3k0s] from comment #3)
> Maybe it would be nice to update them here (bug 750054)

Stephen, should this spinner be updated in Bug 750054 or should we fix it here separately?  It looks like the new spinner mockups at least from http://people.mozilla.org/~shorlander/mockups-interactive/australis-spinner-updates-i02/australis-spinner-updates-02.html doesn't have an inverted state.
Flags: needinfo?(shorlander)
See Also: → bug 750054
I would not worry about bug 750054 for the moment. I would love some consistency but the current ones are outdated and trying to update the ones in the mockup you linked did not work out so well. Hit a serious perf/jank issue.
Flags: needinfo?(shorlander)
(Assignee)

Updated

2 years ago
Whiteboard: [devedition-polish] → [devedition-polish][devedition-40][difficulty=easy]
Whiteboard: [devedition-polish][devedition-40][difficulty=easy] → [devedition-polish][polish-backlog][difficulty=easy]
(Assignee)

Comment 6

2 years ago
(In reply to Stephen Horlander [:shorlander] from comment #1)
> PDFjs has an inverted spinner that we could use:
> http://mxr.mozilla.org/mozilla-central/source/browser/extensions/pdfjs/
> content/web/images/loading-small.png

This icon appears to be 16x17 and not 16x16.  Can you please modify it and send over one with updated dimensions, or let me know if it doesn't matter?

I'm going to switch Nightly/unofficial/Dev Edition brandings to use the inverted spinner since they all have dark backgrounds.
Assignee: nobody → bgrinstead
Status: NEW → ASSIGNED
Flags: needinfo?(shorlander)
(Assignee)

Comment 7

2 years ago
Created attachment 8674336 [details]
MozReview Request: Bug 1173612 - Use inverted loading icon for aboutDialog in Nightly/DevEdition builds;r=Gijs

Bug 1173612 - Use inverted loading icon for aboutDialog in Nightly/DevEdition builds;r=Gijs
Attachment #8674336 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 8

2 years ago
Created attachment 8674338 [details] [diff] [review]
test-apply-checkupdates-local-only.patch

handy patch that forces the 'checking updates' state for local testing
(Assignee)

Comment 9

2 years ago
Created attachment 8674340 [details]
with-patch-unofficial.png
(Assignee)

Comment 10

2 years ago
Created attachment 8674341 [details]
with-patch-devedition.png
(Assignee)

Comment 11

2 years ago
Comment on attachment 8674341 [details]
with-patch-devedition.png

What do you think?  This is using the pdf js icon from Comment 1
Attachment #8674341 - Flags: ui-review?(shorlander)
(Assignee)

Comment 12

2 years ago
Created attachment 8674357 [details]
spinner-comparison.png

comparison before/after patch applied with DE branding active

Comment 13

2 years ago
Comment on attachment 8674336 [details]
MozReview Request: Bug 1173612 - Use inverted loading icon for aboutDialog in Nightly/DevEdition builds;r=Gijs

https://reviewboard.mozilla.org/r/22215/#review19855

LGTM!
Attachment #8674336 - Flags: review?(gijskruitbosch+bugs) → review+
Comment on attachment 8674341 [details]
with-patch-devedition.png

That looks low res to me. There is an @2x version also I think, if not I can make one.

The odd size (16 x 17) is to support the drop shadow. I can make a 16 x 16 version without the drop shadow if needed but I think that one looks fine.
Flags: needinfo?(shorlander)
Attachment #8674341 - Flags: ui-review?(shorlander) → ui-review-

Comment 15

2 years ago
(In reply to Stephen Horlander [:shorlander] from comment #14)
> Comment on attachment 8674341 [details]
> with-patch-devedition.png
> 
> That looks low res to me. There is an @2x version also I think, if not I can
> make one.


https://dxr.mozilla.org/mozilla-central/search?q=path%3Aloading+path%3Atoolkit%2Fthemes&redirect=true&case=true

There is only a low-res one, even for the current throbber. High-res assets for both this and the inverted version would be nice!

Also, Brian, late nit: can you ensure this icon goes in toolkit as well, next to the other ones?
Flags: needinfo?(shorlander)
(Assignee)

Comment 16

2 years ago
Comment on attachment 8674336 [details]
MozReview Request: Bug 1173612 - Use inverted loading icon for aboutDialog in Nightly/DevEdition builds;r=Gijs

Bug 1173612 - Use inverted loading icon for aboutDialog in Nightly/DevEdition builds;r=Gijs
(Assignee)

Comment 17

2 years ago
Created attachment 8674880 [details]
spinner-screenshot@2x.png

I think this should do the trick.. I ran:

cp browser/extensions/pdfjs/content/web/images/loading-small@2x.png browser/themes/shared/loading-inverted@2x.png

and updated the CSS to handle @2x.

Note that the release version is just using a 16px loading icon all the time.  I checked to see if there was a chrome://global/skin/icons/loading_32.png available in toolkit, but there isn't :(
Attachment #8674880 - Flags: ui-review?(shorlander)
(Assignee)

Comment 18

2 years ago
(In reply to :Gijs Kruitbosch from comment #15)
> (In reply to Stephen Horlander [:shorlander] from comment #14)
> > Comment on attachment 8674341 [details]
> > with-patch-devedition.png
> > 
> > That looks low res to me. There is an @2x version also I think, if not I can
> > make one.
> 
> 
> https://dxr.mozilla.org/mozilla-central/
> search?q=path%3Aloading+path%3Atoolkit%2Fthemes&redirect=true&case=true
> 
> There is only a low-res one, even for the current throbber. High-res assets
> for both this and the inverted version would be nice!
> 
> Also, Brian, late nit: can you ensure this icon goes in toolkit as well,
> next to the other ones?

Ah, missed this comment before updating the patch.  Sure, I'll wait for the 32px spinner from shorlander and then move everything into toolkit.
(Assignee)

Updated

2 years ago
Blocks: 1217034
(Assignee)

Comment 19

2 years ago
Filed Bug 1217034 for switching to 2x icons for the release spinners.
Flags: needinfo?(shorlander)
(Assignee)

Comment 20

2 years ago
Comment on attachment 8674880 [details]
spinner-screenshot@2x.png

Got confirmation in #devtools that this looks good
Attachment #8674880 - Flags: ui-review?(shorlander) → ui-review+

Comment 21

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/8c36f375d03b
https://hg.mozilla.org/mozilla-central/rev/8c36f375d03b
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
QA Whiteboard: [good first verify]
Reproduced this bug with Firefox Aurora 40.0a2 (2015-06-10) (Build ID: 20150610004004) on Linux, 64 Bit

This Bug's fix is now verified on Latest Firefox Beta 44.0b7 

Build ID: 20160107144911
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:44.0) Gecko/20100101 Firefox/44.0
QA Whiteboard: [good first verify] → [good first verify][testday-20160108]
I have reproduced this bug with Firefox Developer edition 40.0a2 (Build ID: 20150610004004) on 
windows 8.1 64-bit with the instructions from comment 0.

Verified as fixed with latest Firefox Developer edition 45.0a2 (Build ID: 20160113004011)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0


Verified as fixed with latest Firefox Beta 44.0b8 (Build ID: 20160111185352)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0

As this bug is also verified on Linux (comment 23), I am marking this as verified!
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified
Whiteboard: [devedition-polish][polish-backlog][difficulty=easy] → [devedition-polish][polish-backlog][difficulty=easy][bugday-20160113]
You need to log in before you can comment on or make changes to this bug.