Closed Bug 798212 Opened 7 years ago Closed 7 years ago
flyout may be misaligned horizontally after resizing
The flyout will sometimes such that the right-border moves. As the anchor is on the right hand side, it may become mis-aligned with the anchor
Mark, when does this happen and how should I reproduce it?
(In reply to Asa Dotzler [:asa] from comment #1) > Mark, when does this happen and how should I reproduce it? Hover slowly over the news feed waiting for each item to show the flyout (ie, don't click anywhere - just allow the flyout to open on hover). Eventually you will find a news feed item that is narrower than the "usual" size - when it is displayed the problem can be seen (the anchor arrow will be too far to the left). You may need to scroll to find such an item. Conversely, close the flyout, then start the above process by starting your hover over the one that caused the problem above. Then hover over a different one such that the panel resizes wider. In this case the anchor arrow will be too far to the right and the panel will be partially obscuring the news feed.
Adding [Fx17] as I'm fairly confident that once you see it, you ain't gunna like it :)
Attached is a patch that works around the problem by recording the position of the right-hand-side of the panel before we adjust the size, then moving the panel after the resize such that the right-hand-side ends up in the same position. This is only done for panels with the anchor on the right. If we take this patch, I'll open a new bug to remove this code once the underlying problem is resolved and add a reference to that new bug in a comment.
I applied the patch to see how it would work, and I got a very visual jumping around of the panel, so I wondered if using panel.sizeTo and panel.moveTo would fix it. That didn't, but moving the panel before setting the iframe size does get rid of the visual jumpiness. The previous css based calculations also didn't consider border or padding in content, using getBoundingClientRect allows us to get an accurate box for the body.
Comment on attachment 668745 [details] [diff] [review] Move the panel after resize consider the changes I suggest in the other patch
Attachment #668745 - Flags: feedback?(mixedpuppy) → feedback+
Comment on attachment 668855 [details] [diff] [review] V2 I think we should try and get this patch landed and into Aurora. Note that the change to PANEL_WIN_HEIGHT might cause some flyouts to be too short today, but we can expect changes from the providers such that the calculated size will be correct making the smaller minimum appropriate.
Comment on attachment 668855 [details] [diff] [review] V2 Review of attachment 668855 [details] [diff] [review]: ----------------------------------------------------------------- Reason for not granting r+ is to figure out why the height/width calculations got changed. ::: browser/base/content/browser-social.js @@ +210,5 @@ > return; > } > + let contentRect = doc.body.getBoundingClientRect(); > + > + let height = Math.max(contentRect.height, PANEL_MIN_HEIGHT); Not sure why the height calculation got changed from using marginTop + offsetHeight + marginBottom. This calculation was also changed for the width. Can these changes be reverted? @@ +211,5 @@ > } > + let contentRect = doc.body.getBoundingClientRect(); > + > + let height = Math.max(contentRect.height, PANEL_MIN_HEIGHT); > + let hDiff = height - iframe.boxObject.height; hDiff is unused and as discussed on IRC should be removed.
Attachment #668855 - Flags: review?(jaws) → feedback+
A patch closer to the original, but the move happens before the size, which mixedpuppy reports causes less "shuddering" in his testing.
Comment on attachment 669022 [details] [diff] [review] updated to stick with getComputedStyle and margin sizes Review of attachment 669022 [details] [diff] [review]: ----------------------------------------------------------------- r=me with the one issue below addressed. ::: browser/base/content/browser-social.js @@ +217,3 @@ > let computedWidth = parseInt(cs.marginLeft) + body.offsetWidth + parseInt(cs.marginRight); > let width = Math.max(computedWidth, PANEL_MIN_WIDTH); > + let wDiff = width - iframe.boxObject.width; This comment, https://bugzilla.mozilla.org/show_bug.cgi?id=792295#c6, states that boxObject.width is deprecated and suggests using getBoundingClientRect in its place.
Attachment #669022 - Flags: review?(jaws) → review+
Summary: flyout may be misaligned. → flyout may be misaligned horizontally after resizing
[Approval Request Comment] Bug caused by (feature/regressing bug #): N/A User impact if declined: The social flyout panel may be misaligned horizontally. Testing completed (on m-c, etc.): Tested the alignment problem is resolved. Risk to taking this patch (and alternatives if risky): Risks are limited to social functionality. String or UUID changes made by this patch: None
Attachment #669031 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
You need to log in before you can comment on or make changes to this bug.