1708809 - [Proton Info Bar] Hiding/Closing the Info Bar causes the webpage to move up and down

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P2)

Description

[Mehmet]

Attachment: Demo of webpage moves up and down when closing Info Bar

Nightly 90.0a1 (2021-04-30) (64-Bit)
macOS 11.2.3

1.) Start Nightly
2.) When the Info Bar appears to use Nightly as your Standard Browser click on the hide/close button
3.) take a look at the webpage during the hiding animation

Actual: The Webpage moves up and down.

Expected: No move up/down of the webpage.

A screencast is attached.

Comment 1

[Andrei Oprea [:andreio]]

Hey, thanks for reporting this. I think it's working as expected because we don't want to cover important page content when the infobar shows up.

Comment 2

[Mehmet]

Attachment: screencast_of_bug.mov

(In reply to Andrei Oprea [:andreio] from comment #1)

Hey, thanks for reporting this. I think it's working as expected because we don't want to cover important page content when the infobar shows up.

Thanks for your feedback :)

I think this is a bug, because the whole Toolbar is shifting up and down again when closing the Info Bar. It is more noticeable, when you close the Info Bar in a small window.

Please find attached a new screencast showing the problem again.

Thanks :)

Comment 3

[Andrei Oprea [:andreio]]

I see it now!

Comment 4

[Romain Testard [:RT]]

Within Proton bugs we use P1s to identify release blocker bugs. I'm marking as a high P2 and also assume this is mac only.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Gonna take a crack at this.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1708809 - Make sure the high priority notification box is contained within an element using the CSS box model. r?Gijs!

Mixing the XUL and CSS box models is always a source of interesting glitches like
the one that this patch tries to fix. The animation for closing a Proton-y infobar
involves setting a negative margin on an element using CSS box, which has a parent
and siblings that are using XUL box.

Interestingly, it seems that only the high priority notification box hits this issue,
as the per-tab notification box is contained within a <named-deck>, which uses CSS
box - this observation was what led me to the solution in this patch: when constructing
the high priority notification box, we now place it inside of a <div>, which uses
CSS box, which allows us to sidestep this glitch.

The irony of fixing this CSS vs XUL box glitch by adding another CSS box amongst a
bunch of XUL boxes is not lost on me.

Comment 7

[Peter Magyari (Desktop QA)]

I believe this is a duplicate of Bug 1703182
Shall I set that as a duplicate of this since this one has already been assigned?

Comment 9

[:Gijs (he/him)]

(In reply to Peter from comment #7)

I believe this is a duplicate of Bug 1703182
Shall I set that as a duplicate of this since this one has already been assigned?

Good catch, I've duped that one over to this one as this one has a patch.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/303e9ff76e8c
Make sure the high priority notification box is contained within an element using the CSS box model. r=Gijs

Comment 11

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/303e9ff76e8c

Comment 12

[Shilpa Mohanty]

Request for Uplift from the PM team.

Comment 13

[Peter Magyari (Desktop QA)]

Verified fixed on MacOS 11 using Firefox Nightly 90.0a1 (20210510093555)
Also updating the status flag for Fx 89 since it is also affected

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 9220708 [details]
Bug 1708809 - Make sure the high priority notification box is contained within an element using the CSS box model. r?Gijs!

Beta/Release Uplift Approval Request

• User impact if declined: Users might experience some of their infobars causing minor UI glitches during the close animation.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a very small, self-contained fix that only impacts the high-priority notification box.
• String changes made/needed: None.

Comment 15

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9220708 [details]
Bug 1708809 - Make sure the high priority notification box is contained within an element using the CSS box model. r?Gijs!

Approved for 89 beta 11, thanks.

Comment 16

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/d7eae80a64c0

Comment 17

[Peter Magyari (Desktop QA)]

Verified fixed on Beta 89.0b11 (20210511190154)