Closed Bug 877402 Opened 8 years ago Closed 8 years ago
panel sizing tests did not work correctly
The recent change in bug 799014 didn't take into account the size of the arrowbox when sizing the panel. Our intention is to size the iframe based on the size of content within it. The tests didn't check the iframe size so we didn't catch that the change would size the iframe too small. This patch fixes the iframe sizing as well as the test.
This sets size using the iframe, which is easier than figuring out all the panel margins, arrow size, etc. It also works well for panels like share, which have additional ui elements in the panel. The test has to use the same calculations as the sizeto function in order to validate the iframe is at the correct size.
Assignee: nobody → mixedpuppy
Attachment #755651 - Flags: review?(mhammond)
Comment on attachment 755651 [details] [diff] [review] panel size patch Review of attachment 755651 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/test/social/browser_social_flyout.js @@ +63,1 @@ > // The width of the flyout should be 250px this 250 should be 400 now, right? @@ +64,5 @@ > let iframe = panel.firstChild; > + let body = iframe.contentDocument.body; > + let cs = iframe.contentWindow.getComputedStyle(body); > + // same calculation as in sizeSocialPanelToContent > + let computedWidth = parseInt(cs.marginLeft) + body.offsetWidth + parseInt(cs.marginRight); I don't think there is much value in duplicating the logic here, and having the test use 2 calculated values with a message that says "500px" (or "400px") is confusing. How about just dropping all the computed* values and just using hard-coded 250 and 400 literals?
Attachment #755651 - Flags: review?(mhammond) → review+
more deterministic tests
Comment on attachment 755715 [details] [diff] [review] panel size patch [Approval Request Comment] Bug caused by (feature/regressing bug #): 799014 User impact if declined: social panels are sized too small so content does not appear correctly in panels Testing completed (on m-c, etc.): on m-c Risk to taking this patch (and alternatives if risky): low, only affects social arrow panels String or IDL/UUID changes made by this patch: none
Attachment #755715 - Flags: approval-mozilla-aurora?
Attachment #755715 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 24
It's not clear to me how to get this to apply to Aurora.
Wait, bug 799014 is 24-targeted. Do we actually need this on Aurora? Is this really 23:affected?
(In reply to :Gavin Sharp (use firstname.lastname@example.org for email) from comment #9) > Wait, bug 799014 is 24-targeted. Do we actually need this on Aurora? Is this > really 23:affected? we didn't, my mistake, I had thought bug 799014 made it on 23.
Attachment #755715 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.