Closed Bug 1708809 Opened 7 months ago Closed 7 months ago

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

Categories

(Firefox :: Messaging System, defect, P2)

defect

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox89 --- verified
firefox90 --- verified

People

(Reporter: mehmet.sahin, Assigned: mconley)

References

(Blocks 1 open bug)

Details

(Whiteboard: [proton-infobars] [priority:2a] [proton-uplift])

Attachments

(3 files)

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.

Whiteboard: [proton-infobars]

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.

Attached video 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 :)

I see it now!

Severity: -- → S3
Priority: -- → P1

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.

Priority: P1 → P2
Whiteboard: [proton-infobars] → [proton-infobars] [priority:2a]

Gonna take a crack at this.

Assignee: nobody → mconley

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.

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?

Flags: needinfo?(mconley)
Duplicate of this bug: 1703182

(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.

Flags: needinfo?(mconley)
Pushed by mconley@mozilla.com:
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
See Also: → 1710092
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

Request for Uplift from the PM team.

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 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.
Attachment #9220708 - Flags: approval-mozilla-beta?

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.

Attachment #9220708 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [proton-infobars] [priority:2a] → [proton-infobars] [priority:2a] [proton-uplift]

Verified fixed on Beta 89.0b11 (20210511190154)

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.