Closed Bug 1324280 Opened 3 years ago Closed 3 years ago

Website certificate superimposed over navigation back/forward

Categories

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

53 Branch
All
Android
defect
Not set
major

Tracking

()

RESOLVED WORKSFORME
Tracking Status
fennec 53+ ---
firefox50 --- ?
firefox51 --- ?
firefox52 --- ?
firefox53 --- affected

People

(Reporter: davross, Assigned: maliu)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

Attached image navigation.png
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20161217030205

Steps to reproduce:

Using Nightly on any website. After some activity the Tracking Protection/Website SSL Security Cert ends up superimposed over the top of the navigation buttons. Pretty consistent reproduction by simply going 'back' with my device native back key.


Actual results:

Clash of the icons


Expected results:

Everything should behave, just not today. Normal browser behaviour is what's expected.
From the URL bar it looks like this is happening on a tablet? I do not see this on my N9. What device and Android version are you using?
I can reproduce this issue using Asus ZenPad 8 (Android 6.0.1) and Asus Transformer Pad (Android 4.4.1).
On Nexus 9, I can't reproduce the issue.
Since I have no more tablet devices to test on, I can only assume that this is an Asus specific issue.
Flags: needinfo?(sw1ayfe)
tracking-fennec: --- → ?
Is this a regression? What versions are affected?
This is a Samsung Galaxy Tab A running Android 6.0.1 (Marshmallow). Behaviour only experienced in Nightly Fennec 53 so far (and occurring irregularly), but have downloaded 52, 51, and 50. Will test further. Odd behaviour with these icon has it spaced over to the right when not layered on top of each other. Uploading image 'RIGHT'
Flags: needinfo?(sw1ayfe)
Max: Did the RTL patches touch the tablet toolbar and could have caused this?
Flags: needinfo?(max)
(In reply to Sebastian Kaspari (:sebastian) from comment #6)
> Max: Did the RTL patches touch the tablet toolbar and could have caused this?
It's possible caused by RTL patches. Though similar symptom only happen on RTL context, I'll take it.
Assignee: nobody → max
Flags: needinfo?(max)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Status: NEW → ASSIGNED
I just changed this to major.  this looks really ugly.  I don't think a release with this behavior can go forward.
Severity: normal → major
tracking-fennec: ? → 53+
Attachment #8820585 - Flags: review?(s.kaspari)
Comment on attachment 8820585 [details]
Bug 1324280 - Revert MarginLayoutParamsCompat with setting margin left/right by current layout direction,

https://reviewboard.mozilla.org/r/100078/#review100978

::: mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbarTablet.java:13
(Diff revision 2)
> +import org.mozilla.gecko.R;
> +import org.mozilla.gecko.animation.PropertyAnimator;
> +import org.mozilla.gecko.animation.ViewHelper;
> +

Seems like those lines moved around. Let's try to not do this to avoid conflicts with other patches/uplifts. :)

::: mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbarTablet.java:101
(Diff revision 2)
> -                    MarginLayoutParamsCompat.setMarginStart(layoutParams, 0);
> +                    if (isLayoutRtl()) {
> +                        layoutParams.rightMargin = 0;
> +                    } else {
> +                        layoutParams.leftMargin = 0;
> +                    }

I thought that's what MarginLayoutParamsCompat does internally for older versions? But here we want to always update the left/right margin?
A comment in the bug / commit message / code would be helpful to help me and others understand. :)

::: mobile/android/base/java/org/mozilla/gecko/toolbar/BrowserToolbarTablet.java:109
(Diff revision 2)
> -                    MarginLayoutParamsCompat.setMarginStart(layoutParams, 0);
> +                    if (isLayoutRtl()) {
> +                        layoutParams.rightMargin = 0;
> +                    } else {
> +                        layoutParams.leftMargin = 0;
> +                    }

This pattern is repeated multiple times - do we want a helper method like the one in the compat class?
Attachment #8820585 - Flags: review?(s.kaspari) → review+
Comment on attachment 8820585 [details]
Bug 1324280 - Revert MarginLayoutParamsCompat with setting margin left/right by current layout direction,

https://reviewboard.mozilla.org/r/100078/#review101026
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4d1a2b1de7f5
Revert MarginLayoutParamsCompat with setting margin left/right by current layout direction, r=sebastian
Keywords: checkin-needed
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/8d50229a6812
Revert MarginLayoutParamsCompat with setting margin left/right by current layout direction: lint style fixes. r=lint-fixes
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Using Nightly (2017-01-03) this is not resolved. Forgive my ignorance, is there delay before pushed to Nightly? (See new attachment navbug.png)
Flags: needinfo?(max)
Attached image navbug.png
Bug persistent in Nightly (2017-01-03)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: needinfo?(max)
Hi David,

Patch landed days ago and I have verify latest nightly(2017-01-03) pass on emulator from platform 17 to 24. Seems symptom remains on 23(marshmallow) only.

May I confirm what device/platform are you using please? Thank you.
Flags: needinfo?(sw1ayfe)
Depends on: 1328515
Samsung 'Galaxy Tab A' SM-T555 running Android Marshmallow 6.0.1 (Build number MMB29M.T555XXU1BPDA)
Flags: needinfo?(sw1ayfe) → needinfo?(max)
Latest Nightly should include the solution patch. Please give it a try. Thank you.
Flags: needinfo?(max)
Wahoo! Thanks a bunch everyone. Excellent job. That was such a frustrating bug for me.
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → WORKSFORME
Blocks: rtl-meta
OS: Unspecified → Android
Hardware: Unspecified → All
Duplicate of this bug: 1322142
You need to log in before you can comment on or make changes to this bug.