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

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Brian Grinstead [:bgrins]]

Attachment: 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.

Comment 1

[Stephen Horlander [:shorlander]]

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

Comment 2

[Stephen Horlander [:shorlander]]

This also applies to Nightly.

Comment 3

[u428464]

Maybe it would be nice to update them here (bug 750054)

Comment 4

[Brian Grinstead [:bgrins]]

(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.

Comment 5

[Stephen Horlander [:shorlander]]

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.

Comment 6

[Brian Grinstead [:bgrins]]

(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.

Comment 7

[Brian Grinstead [:bgrins]]

Attachment: 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

Comment 8

[Brian Grinstead [:bgrins]]

Attachment: test-apply-checkupdates-local-only.patch

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

Comment 9

[Brian Grinstead [:bgrins]]

Attachment: with-patch-unofficial.png

Comment 10

[Brian Grinstead [:bgrins]]

Attachment: with-patch-devedition.png

Comment 11

[Brian Grinstead [:bgrins]]

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

What do you think?  This is using the pdf js icon from Comment 1

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: spinner-comparison.png

comparison before/after patch applied with DE branding active

Comment 13

[:Gijs (he/him)]

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!

Comment 14

[Stephen Horlander [:shorlander]]

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.

Comment 15

[:Gijs (he/him)]

(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?

Comment 16

[Brian Grinstead [:bgrins]]

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

Comment 17

[Brian Grinstead [:bgrins]]

Attachment: [e-mail]

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

cp browser/extensions/pdfjs/content/web/images/[e-mail] browser/themes/shared/[e-mail]

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 :(

Comment 18

[Brian Grinstead [:bgrins]]

(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.

Comment 19

[Brian Grinstead [:bgrins]]

Filed Bug 1217034 for switching to 2x icons for the release spinners.

Comment 20

[Brian Grinstead [:bgrins]]

Comment on attachment 8674880 [details]
[e-mail]

Got confirmation in #devtools that this looks good

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8c36f375d03b

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8c36f375d03b

Comment 23

[Nazir Ahmed Sabbir [:NaSb]]

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

Comment 24

[Mohammad Maruf Islam]

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!