1390620 - Botched rebase: address (if needed) overrideScaleType and fitCenter for StreamPageIconLayout

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, enhancement, P1)

Description

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

In my rebase of bug 1301718, I needed to rebase over Sebastian's favicon changes that changed `overrideScaleType=false` and `scaleType=fitCenter` on the faviconView. However, I forgot to do the update during the rebase and now there's a TODO in the code.

Verify it's actually a problem and fix it!

Comment 1

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

bug 1383736 was the bug that Sebastian made the favicon changes in and here's a link to the lines:

https://hg.mozilla.org/mozilla-central/annotate/b95b1638db48fc3d450b95b98da6bcd2f9326d2f/mobile/android/app/src/main/res/layout/activity_stream_card_history_item.xml#l29

Comment 2

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

Note: all I landed was highlights changes so this should only affect the highlights icons.

Comment 3

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

Attachment: Bug 1390620: Set centerInside StreamPageIconLayout's FaviconView; add comments.

afaict, in practice, I think the favicons we create are all sized to the
FaviconView so this change has no effect. However, in theory, it is more correct
to constrain the favicons by fitCenter.

Review commit: https://reviewboard.mozilla.org/r/168832/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/168832/

Comment 4

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

Comment on attachment 8897559 [details]
Bug 1390620: Set centerInside StreamPageIconLayout's FaviconView; add comments.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168832/diff/1-2/

Comment 5

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

Comment on attachment 8897559 [details]
Bug 1390620: Set centerInside StreamPageIconLayout's FaviconView; add comments.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/168832/diff/2-3/

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8897559 [details]
Bug 1390620: Set centerInside StreamPageIconLayout's FaviconView; add comments.

https://reviewboard.mozilla.org/r/168832/#review175854

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ecc3f03e5eae
Set centerInside StreamPageIconLayout's FaviconView; add comments. r=sebastian

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ecc3f03e5eae