[Protections Panel] descriptionHeightWorkaround is not applied in time to all necessary elements, resulting in clipping
Categories
(Firefox :: Site Identity, defect, P1)
Tracking
()
People
(Reporter: obotisan, Assigned: nhnt11)
References
(Blocks 1 open bug)
Details
(Whiteboard: [privacy-panel][skyline])
Attachments
(3 files)
117.13 KB,
image/png
|
Details | |
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
47 bytes,
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details | Review |
Affected versions
- Firefox 71.a01
- Firefox 70.0b8
Affected platforms
- Windows 10 x64
- Ubuntu 18.04 x64
- macOS 10.14
Steps to reproduce
- Place the browser on the lower half of the screen.
- Open any site with a new profile or click on the shield icon from URL bar and then on the "i" symbol.
- Observe the "Browse without being followed" panel.
Expected result
- The doorhanger is correctly displayed .
Actual result
- Part of the doorhanger is missing.
Regression range
- This is a not regression.
Additional notes
- Please look at the attached image.
- This happens sometimes when you open the browser for the first time. I just logged the issue under this steps, because following these steps the issue is always reproducing.
Updated•5 years ago
|
Assignee | ||
Comment 2•5 years ago
•
|
||
I managed to reproduce this bug and figure out the issue.
STR:
- Open a new window (or restart/open Firefox - basically you must use a window in which the protections popup has never been opened)
- Navigate to reddit.com, and click the shield icon as soon as it's available, before the site finishes loading
- Wait for the site to finish loading - the panel will go from the "no trackers detected" state to a state where it shows category items. At this point, the content overflows the panel and it doesn't resize, resulting in clipping.
I debugged this by adding a console.log() in descriptionHeightWorkaround()
when it actually sets the heights of elements. The first time the panel is opened, all the category section items are omitted, because at the time they are processed, they are invisible.
The solution is to call descriptionHeightWorkaround() manually, the first time we show the category items. Patch is coming up.
Assignee | ||
Comment 3•5 years ago
|
||
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/35ca0dc8f9e7 Manually invoke descriptionHeightWorkaround() on protections panel the first time we show category items. r=johannh
Comment 5•5 years ago
|
||
bugherder |
Assignee | ||
Comment 6•5 years ago
|
||
Comment on attachment 9097443 [details]
Bug 1582750 - Manually invoke descriptionHeightWorkaround() on protections panel the first time we show category items. r=johannh
Beta/Release Uplift Approval Request
- User impact if declined: Occasional visual glitch - especially on first run of 70 when the onboarding message is shown.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: See comment 2
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): The patch adds an extra call to an existing function that is already called every time the panel is opened. The function in question implements caching and does nothing if the input data didn't change. The patch is an objective improvement and even if there turns out to be a case that isn't covered, this patch should do no harm.
- String changes made/needed:
Assignee | ||
Updated•5 years ago
|
Comment 7•5 years ago
|
||
Comment on attachment 9097443 [details]
Bug 1582750 - Manually invoke descriptionHeightWorkaround() on protections panel the first time we show category items. r=johannh
Polish for Skyline feature, fine for uplift for beta 12.
Assignee | ||
Comment 8•5 years ago
|
||
This is going to need a further adjustment. There's one edge case it doesn't cover, here's the state flow for it:
- User begins a page load.
- User opens protections popup, it says "No trackers detected."
- Content blocking event received, but only Allowing and None Detected - nothing Blocked.
- Another content blocking event received, this time all three shown.
We call descriptionHeightWorkaround
at step 3, but we also need to call it at step 4 - and we don't.
The fix is to call eliminate our "_descriptionHeightWorkaroundCalled" (or whatever I named it) state variable, and simply call it every time. And I think this is necessary and OK.
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 9•5 years ago
|
||
Comment 10•5 years ago
|
||
bugherder uplift |
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Pushed by nhnt11@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5f9ffb33e44e Ensure descriptionHeightWorkaround is called for multiple consecutive content blocking events. r=johannh
Comment 12•5 years ago
|
||
bugherder |
Assignee | ||
Comment 13•5 years ago
|
||
Comment on attachment 9097783 [details]
Bug 1582750 - Ensure descriptionHeightWorkaround is called for multiple consecutive content blocking events. r=johannh
Beta/Release Uplift Approval Request
- User impact if declined: See comment 6. Visual polish/quality.
- Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: No
- If yes, steps to reproduce: See comment 8
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Small tweak to confirmed fix - covers an edge case.
- String changes made/needed:
Comment 14•5 years ago
|
||
Bugbug thinks this bug is a regression, but please revert this change in case of error.
Reporter | ||
Comment 15•5 years ago
|
||
We managed to investigate the issue a bit and these are the results:
STR from comment 2:
- using latest Nightly 71.0a1 and Firefox 70.0b12 on Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64. The issue is not reproducing anymore.
STR from comment 0:
- I verified the fix using latest Nightly 71.0a1 and Firefox 70.0b12 on Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64. The issue is still reproducing.
Should I log a different bug with the steps from comment 0? Maybe an enhancement for that part?
Comment 16•5 years ago
|
||
Comment on attachment 9097783 [details]
Bug 1582750 - Ensure descriptionHeightWorkaround is called for multiple consecutive content blocking events. r=johannh
Followup fix for skyline feature. OK for beta 13 uplift.
Updated•5 years ago
|
Comment 18•5 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 19•5 years ago
|
||
I logged bug 1586652 as a follow up bug.
Assignee | ||
Comment 20•5 years ago
|
||
Updated•5 years ago
|
Comment 21•5 years ago
|
||
I cannot reproduce the steps from comment 2 anymore, on latest Nightly 71.0a1 and Beta 70.0b13 across platforms: Windows 10 x64, macOS 10.13, Ubuntu 18.04 x64.
The initial steps reported in the bug (comment 0) are still repro, but since the issue is tracked separately in bug 1586652, I think we can close this one. Marking as verified fixed.
Updated•5 years ago
|
Description
•