1063914 - Restore edit mode's [x] icon

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Lucas Rocha (:lucasr)]

Regression from bug 1054916 I think.

Comment 1

[Lucas Rocha (:lucasr)]

Attachment: Restore edit mode's [x] icons (r=mcomella)

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8487212 [details] [diff] [review]
Restore edit mode's [x] icons (r=mcomella)

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

What did you do to modify the image files?

Comment 3

[Lucas Rocha (:lucasr)]

(In reply to Michael Comella (:mcomella) from comment #2)
> Comment on attachment 8487212 [details] [diff] [review]
> Restore edit mode's [x] icons (r=mcomella)
> 
> Review of attachment 8487212 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> What did you do to modify the image files?

Simply re-committed the original images I used in bug 1010740.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8487212 [details] [diff] [review]
Restore edit mode's [x] icons (r=mcomella)

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

wfm, but I'm afraid of the loss of the optimizations we were supposed to make in bug 1054916.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Chris, is there anything to be done here to restore the optimizations from bug 1054916?

Comment 6

[Chris Kitching [:ckitching]]

(In reply to Michael Comella (:mcomella) from comment #5)
> Chris, is there anything to be done here to restore the optimizations from
> bug 1054916?

Of course, just run `trimage` on all the PNGs you touched.

It's in the AUR.

Usage:

`trimage -f something.png`

Simplez!

Comment 7

[Chris Kitching [:ckitching]]

Oh. Right. I should read context. That caused the bug.... Wat.

Sec.

Comment 8

[Chris Kitching [:ckitching]]

So these images look identical to the original ones. However, trimage uses indexed PNGs with an alpha channel, something which seems to be poorly supported.
Perhaps Android is incapable of loading such PNGs?

I'm pretty unclear how two images that are ostensibly identical can look different. Is the symptom that the image fails to load at all, or that it loads broken in some way?

Comment 9

[Lucas Rocha (:lucasr)]

Context: I changed the icons in bug 1010740 but bug 1054916 landed in m-c after it. The patch in bug 1054916 was applied on the old icons and overrode the images I had added. The main evidence of this is that the xxhdpi version of the new icon (from bug 1010740) wasn't affected because it didn't exist by the time Chris generated the patch for bug 1054916.

Anyway, I did run trimage on the images I restored here. They're all properly optimized.

Comment 10

[Chris Kitching [:ckitching]]

Oh, that's great news. Not a problem with trimage , just an annoying race condition. Sorry for the confusion.
Go forth and land it! :P

Comment 11

[Lucas Rocha (:lucasr)]

https://hg.mozilla.org/integration/fx-team/rev/5cfb23b1d315

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5cfb23b1d315