All users were logged out of Bugzilla on October 13th, 2018

Activity stream - Favicons and context menu icons are wrongly displayed

VERIFIED FIXED in Firefox 54

Status

()

P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: carmenf, Assigned: ahunt)

Tracking

53 Branch
Firefox 54
ARM
Android
Points:
---

Firefox Tracking Flags

(firefox53 affected, firefox54 verified)

Details

(Whiteboard: [MobileAS])

Attachments

(4 attachments)

Created attachment 8820753 [details]
Asus Transformer Pad UI.png

Environment: 
Device:  Asus Transformer Pad (Android 4.2.1);
Build: latest Nightly 53.0a1 (2016-12-20);

Steps to reproduce:
1. Launch Firefox;
2. Observe the UI of Activity stream panel.

Expected result:
Activity stream panel is properly displayed.

Actual result:
- Favicons from the Highlights area are displayed on the down side in the website’s frame;
- Context menu icon is bigger than it should be and it’s displayed to the limit of the website’s frame. 

Notes:
This only happens on Asus Transformer Pad, on Nexus 9 (Android 7.0) and Asus ZenPad (6.0.1) the Activity stream panel is displayed properly.
Please see the screenshot attached.

Regression range:

Last good build: 2016-11-11
First bad build: 2016-11-12

Pushlog: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d38d06f85ef59c5dbb5d4a1a8d895957a78714de&tochange=b37be3d705d929ee52280051d58cedc70a47626f
(Reporter)

Updated

2 years ago
Whiteboard: [MobileAS]
Priority: -- → P2
(Assignee)

Comment 1

2 years ago
I'm able to reproduce this on a TF201T ("Transfomer Prime"), so I could debug/fix locally.

I don't have any Asus phones locally, so I've got no idea if this is specific to Asus tablets, or a generic Asus bug.
(In reply to Andrzej Hunt :ahunt from comment #1)
> I'm able to reproduce this on a TF201T ("Transfomer Prime"), so I could
> debug/fix locally.

That would be perfect.

> I don't have any Asus phones locally, so I've got no idea if this is
> specific to Asus tablets, or a generic Asus bug.

Maybe it comes apparent when you know the reason for it. But according to comment 0 it works on an Asus ZenPad. Maybe it's an Android 4 + Asus thing?
(Assignee)

Comment 3

2 years ago
Also reproduceable on a Motorola Droid 4 running 4.1.2 (which is a phone).
(Assignee)

Updated

2 years ago
Assignee: nobody → ahunt
Iteration: --- → 1.14
Priority: P2 → P1
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 7

2 years ago
mozreview-review
Comment on attachment 8829922 [details]
Bug 1325096 - Fix menu button padding on select devices

https://reviewboard.mozilla.org/r/106886/#review108166

nit: Maybe seperate padding preserve things from enableTouchRipple to make it reusable for other view reference to setBackgroundDrawable. See comment on 3/3 patch
Attachment #8829922 - Flags: review?(max) → review+

Comment 8

2 years ago
mozreview-review
Comment on attachment 8829923 [details]
Bug 1325096 - Wrap FaviconView to workaround ignored margin

https://reviewboard.mozilla.org/r/106888/#review108172

LGTM
Attachment #8829923 - Flags: review?(max) → review+

Comment 9

2 years ago
mozreview-review
Comment on attachment 8829924 [details]
Bug 1325096 - Post: only use deprecated setBackgroundDrawable on older platforms

https://reviewboard.mozilla.org/r/106890/#review108158

::: mobile/android/base/java/org/mozilla/gecko/util/ViewUtil.java:67
(Diff revision 1)
>          // This call is deprecated, but the replacement setBackground(Drawable) isn't available
>          // until API 16.
> -        view.setBackgroundDrawable(backgroundDrawableArray.getDrawable(0));
> +        if (AppConstants.Versions.feature16Plus) {
> +            view.setBackground(backgroundDrawable);
> +        } else {
> +            view.setBackgroundDrawable(backgroundDrawable);
> +        }

nit: Since padding lost is a framework bug cound happen to any where else, how about wrap this block with padding preserve and new api compatible together?

```java
    @TargetApi(Build.VERSION_CODES.JELLY_BEAN)
    public static void setBackground(View view, Drawable drawable) {
        // On certain older devices (e.g. ASUS TF200T, Motorola Droid 4), setting a background
        // drawable results in the padding getting wiped. We work around this by saving the pre-existing
        // padding, followed by restoring it at the end in view.setPadding().

        final int paddingLeft = view.getPaddingLeft();
        final int paddingTop = view.getPaddingTop();
        final int paddingRight = view.getPaddingRight();
        final int paddingBottom = view.getPaddingBottom();

        if(AppConstants.Versions.preJB) {
            // This call is deprecated, but the replacement setBackground(Drawable) isn't available
            // until API 16.
            view.setBackgroundDrawable(drawable);
        } else {
            view.setBackground(drawable);
        }

        if (AppConstants.Versions.preLollipop) {
            view.setPadding(paddingLeft, paddingTop, paddingRight, paddingBottom);
        }
    }
```
Attachment #8829924 - Flags: review?(max) → review+

Comment 10

2 years ago
mozreview-review
Comment on attachment 8829924 [details]
Bug 1325096 - Post: only use deprecated setBackgroundDrawable on older platforms

https://reviewboard.mozilla.org/r/106890/#review108468

::: mobile/android/base/java/org/mozilla/gecko/util/ViewUtil.java:59
(Diff revision 1)
>          }
>      }
>  
> +    @TargetApi(Build.VERSION_CODES.JELLY_BEAN)
>      private static void setTouchRipple(final View view) {
>          final TypedArray backgroundDrawableArray;

nit: backgroundDrawableArray.recycle()
(Assignee)

Comment 11

2 years ago
mozreview-review
Comment on attachment 8829924 [details]
Bug 1325096 - Post: only use deprecated setBackgroundDrawable on older platforms

https://reviewboard.mozilla.org/r/106890/#review109662

::: mobile/android/base/java/org/mozilla/gecko/util/ViewUtil.java:59
(Diff revision 1)
>          }
>      }
>  
> +    @TargetApi(Build.VERSION_CODES.JELLY_BEAN)
>      private static void setTouchRipple(final View view) {
>          final TypedArray backgroundDrawableArray;

BTW feel free to use the "open an issue" checkbox: that then forces the patch author to "fix" (or drop) the issue before pushing, which helps ensure that your comment gets addressed!
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 15

2 years ago
Pushed by ahunt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e13b9d48a99a
Fix menu button padding on select devices r=maliu
https://hg.mozilla.org/integration/autoland/rev/bbde80c84235
Wrap FaviconView to workaround ignored margin r=maliu
https://hg.mozilla.org/integration/autoland/rev/83dc8e87b382
Post: only use deprecated setBackgroundDrawable on older platforms r=maliu

Comment 16

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/e13b9d48a99a
https://hg.mozilla.org/mozilla-central/rev/bbde80c84235
https://hg.mozilla.org/mozilla-central/rev/83dc8e87b382
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54

Comment 17

2 years ago
Verified as fixed on Nightly 54.0a1 (2017-2-2), on the following devices:
- Asus Transformer Pad TF300T (Android 4.2.1)
- Motorola Razr (Android 4.4.4)
- Lenovo Yoga Tablet 2 (Android 4.4.2)
- Samsung Galaxy Note 4 (Android 5.0.1)
Status: RESOLVED → VERIFIED
status-firefox54: fixed → verified
You need to log in before you can comment on or make changes to this bug.