Closed Bug 1330986 Opened 3 years ago Closed 3 years ago

Activity stream - There are some missing icons and not properly rendered few ones on context menu

Categories

(Firefox for Android :: General, defect, P1)

53 Branch
ARM
Android
defect

Tracking

()

VERIFIED FIXED
Firefox 53
Iteration:
1.13
Tracking Status
firefox53 --- fixed
firefox54 --- verified

People

(Reporter: carmenf, Assigned: ahunt)

References

Details

(Whiteboard: [MobileAS])

Attachments

(4 files)

Attached image Missing icons.png
Environment: 
Device:  Motorola Razr xt910 (Android 4.4.4);
Build: Nightly 53.0a1 (2017-01-12);

Prerequisites: Activity stream enabled.

Steps to reproduce:
1. Launch Firefox;
2. Tap on the context menu icon on a website either from Top sites area or Highlights(3 dot menu);
3. Observe the icons of the context menu's options.

Expected result:
Icons from the options are properly displayed.

Actual result:
Some of the icons are not displayed;
Few icons that are displayed are not properly rendered. 

Notes:
Please see the screenshot attached.
This also happens on landscape mode.
On Nexus 5 (Android 6.0.1) and Prestigio Grace 5x (Android 4.4.2) this is not reproducible.
Whiteboard: [MobileAS]
Priority: -- → P2
ahunt: Could this be the rendering of vector drawables failing on this device? Would be a pity to go back to PNGs.
Flags: needinfo?(ahunt)
Interestingly, the icons that are actually corrupted, are the same ones that get corrupted in the 6.0-specific Bug (Bug 1325929). I'd be interested in knowing if that disappears once that fix lands.

We're not the only people seeing this type of corruption, so it does seem VectorDrawable specific:
https://code.google.com/p/android/issues/detail?id=209558

I'm not quite sure why some icons are completely missing, but it's possible that if there's enough corruption in the right places, then icons can disappear (at least one person has a screenshot with a missing icon in that bug).

*if* landing Bug 1325929 fixes the issue, then I can try and perform the same cleanup on the remaining icons, otherwise I'll keep digging.
Tested again on latest Nightly build 53.0a1 (2017-01-15) using Motorola Razr xt910 (Android 4.4.4) but there are still icons that are not displayed and few of those who are displayed are not properly rendered.

(In reply to Andrzej Hunt :ahunt from comment #2)
> Interestingly, the icons that are actually corrupted, are the same ones that
> get corrupted in the 6.0-specific Bug (Bug 1325929). I'd be interested in
> knowing if that disappears once that fix lands.

Not only the 3 icons from Bug 1325929 are corrupted, but all of them are.
Also, I have noticed that those 3 affected icons are more frequently displayed than the others.
Please see the following screenshot and notice that all the context menu icons have problems in rendering when displayed: http://imgur.com/a/uenm9
It turns out I can reproduce aspects of this on an N4 running 4.4.4 too, but only on Nightly, and not in my own builds.

That suggests we're probably hitting the following Android bug, which only affects proguarded builds (which explains why nightly is affected, but not local builds):
https://code.google.com/p/android/issues/detail?id=209558#makechanges

There's a workaround which I'll test. There's a claim that "newer" versions of the support library will address this, but that doesn't appear to be verified anywhere - and I doubt we'll be in a position to use 25.X support libraries anytime soon regardless.
Flags: needinfo?(ahunt)
(In reply to Andrzej Hunt :ahunt from comment #4)
> It turns out I can reproduce aspects of this on an N4 running 4.4.4 too, but
> only on Nightly, and not in my own builds.

And I forgot to mention what I'm seeing: I can see missing icons (the first time I opened the menu it was fine, closing and reopening results in different icons missing every time). I occasionally see corruption too (on the icons that are rendered), it's not consistent and doesn't happen every time.
Comment on attachment 8827405 [details]
Bug 1330986 - Remove all remaining VectorDrawable arc curves

https://reviewboard.mozilla.org/r/105090/#review105902

In case this works: r+ - Just read the issue on b.android.com and thought I could review the patch while I'm here. :)
Attachment #8827405 - Flags: review+
My first attempt didn't work, for my second attempt I also added
' proguard "-keepattributes LocalVariableTable"'

That has resulted in the corruption disappearing and all icons working. I don't see any increase in APK size when adding the LocalVariableTable line. (In fact I see a 10kb decrease). 

What I also noticed is that the share icon didn't seem to have issues: both in nightly, and in the patched try-built versions the icon never shows any corruption, and doesn't disappear. One person on the google bug mentioned that arc paths seem to cause issues: the share icon doesn't have any, other icons do.

I was able to remove arc paths in a local test either by:
- Opening in boxy SVG, ungroup, save
- inkscape FILENAME.svg --verb=EditSelectAll --verb=SelectionSimplify --verb=FileSave

I haven't actually tested the output SVGs yet, so we can't be certain that the above process actually helps. Since the proguard changes are enough to get icons working I think we should stick with that, which saves us from having to convert new SVG icons every single time we want to add them to the app.
Assignee: nobody → ahunt
Status: NEW → ASSIGNED
Iteration: --- → 1.13
Priority: P2 → P1
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/17f52807a316
Override proguard config to fix VectorDrawable corruption on Android 4 r=sebastian
https://hg.mozilla.org/mozilla-central/rev/17f52807a316
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
The issue is still reproducible on Motorola Razr (Android 4.4.4) & Asus Transformer Pad TF300T (Android 4.2.1). 
note: Does not reproduce on first try, it will reproduce after a few activities are added to highlights.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Ughh. It looks like the icons that fail still have arc curves (which I'd partially fixed in Bug 1325929).

Hopefully converting the remaining icons should fix them (I've tested repeatedly on the TF200T: the bookmark, share, and private tab icons are always rendered correctly - they also have no arc curves. The other icons occasionally fail, and do contain arc curves.)
(Note to self: if the letter A or A is contained somewhere within the android:pathData string, the VectorDrawable has an arc curve.)
Comment on attachment 8832968 [details]
Bug 1330986 - Fix filename typo

https://reviewboard.mozilla.org/r/109206/#review110352
Attachment #8832968 - Flags: review?(s.kaspari) → review+
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/370e3956a238
Remove all remaining VectorDrawable arc curves r=sebastian
https://hg.mozilla.org/integration/autoland/rev/2e9ca1cb6df9
Fix filename typo r=sebastian
https://hg.mozilla.org/mozilla-central/rev/370e3956a238
https://hg.mozilla.org/mozilla-central/rev/2e9ca1cb6df9
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Verified as fixed on the latest Nightly 54 (2017-02-05).
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.