1311533 - White border around AS topsites on Android 4

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect, P1)

Description

[Andrzej Hunt :ahunt]

Attachment: topsites_border_cropped.png

On my current build I see a white border around the favicon on Android 4 (I haven't tested on newer devices, but I'm guessing they work fine). Screenshot of the problem is attached, see tile 2 and 4.

This seems to be caused by Bug 1308525 which switched TopSitesCard from FilledCardView to CardView.

Comment 1

[Andrzej Hunt :ahunt]

Attachment: topsites_filledcardview_cropped.png

This is what it would look like with FilledCardView on Android 4 (note: Android >= 5 would not change, we only have to disable corners on Android 4 because of platform limitations).

Comment 2

[Andrzej Hunt :ahunt]

Just to check: was switching to CardView in Bug 1308525 deliberate, or do we specifically want rounder corners (which causes the border) on Android 4?

Comment 3

[Andrzej Hunt :ahunt]

Attachment: Bug 1311533 - Switch Activity Stream topsites card back to FilledCardView

This ensures no white borders around the favicons when on Android 4, no changes
on Android 5. Read FilledCardView for the full background on the border/corner
issue that happens on Android <= 4.

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

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Andrzej Hunt :ahunt from comment #2)
> Just to check: was switching to CardView in Bug 1308525 deliberate, or do we
> specifically want rounder corners (which causes the border) on Android 4?

Oops, no this wasn't deliberate. I remember replacing it with a FrameLayout because I was debugging some padding/margin issues, but then I probably have changed it back to CardLayout instead of FilledCardView.

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8802708 [details]
Bug 1311533 - Switch Activity Stream topsites card back to FilledCardView

https://reviewboard.mozilla.org/r/87028/#review86132

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7dfb3865e126
Switch Activity Stream topsites card back to FilledCardView r=sebastian

Comment 7

[Guilherme Lima]

Landed 22 hours ago https://hg.mozilla.org/mozilla-central/rev/7dfb3865e126