Closed Bug 524545 Opened 10 years ago Closed 9 years ago
A popup opened with large margins appears offscreen ("ASSERTION: Popup is offscreen")
This causes an assertion and the popup isn't visible because it is offscreen.
Summary: Popups opened with large margins or using openPopupAtScreen(largeNumber, largeNumber) appear offscreen → A popup opened with large margins appears offscreen
This patch: - after flipping or moving the popup, makes sure the position and size are within the screen (or content area) boundaries. - fixes the case clauses so that invalid values of popupAnchor or popupAlign are just treated as topleft. I found this while writing the test and caused it to use random values at times. Note that this bug only occurs when large margins are used on the popup which push the popup offscreen, or cause its size to be negative.
Attachment #408843 - Flags: review?(roc)
Attachment #408843 - Flags: review?(roc) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Looks like this checkin caused some orange: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257284533.1257285163.28678.gz There are 8 test-failures like this: 8331 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_popup_attribute.xul | open popup with large positive margin y position before_start - got 0, expected 406 And 8 test-failures like this: 9028 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_popup_button.xul | open popup with large positive margin y position before_start - got 0, expected 406 Backing out in a minute, unless Neil responds on IRC or in this bug... :)
(In reply to comment #3) > Backing out in a minute, unless Neil responds on IRC or in this bug... :) Ah, nevermind -- Neil just backed out: http://hg.mozilla.org/mozilla-central/rev/ee113c74758e
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Looks like Neil relanded: http://hg.mozilla.org/mozilla-central/rev/7d7f59076479 But the same failures are showing up again on Mac, e.g.: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1257810963.1257811323.26188.gz I will back out.
Comment on attachment 497168 [details] [diff] [review] Updated patch >+ // Make sure that the point in within the screen boundaries and that the Nit: s/in/is/
Neil, what is the next step here?
"blocking2.0=?": Bug 616607 was blocking2.0+. This is "blocking" work on bug 626563 that causes a perma-orange on SeaMonkey.
blocking2.0: --- → ?
I would happily approve this patch if it comes through, but I don't think it blocks the release (as bug 626563 doesn't either)
blocking2.0: ? → -
This patch just adjusts the offsets in the test a bit. It doesn't change any non-test code from the previously reviewed patch.
Attachment #508507 - Flags: approval2.0?
Attachment #508507 - Flags: approval2.0? → approval2.0+
Summary: A popup opened with large margins appears offscreen → A popup opened with large margins appears offscreen ("ASSERTION: Popup is offscreen")
This should land; I'm happy to preserve the approval here. Can we get 'er done?
It's not clear which attachment should be landed here. Please generate a single patch for landing, and mark everything else as obsolete.
Whiteboard: [patchlove][needs-landing] → [patchlove]
None of it should be. Patches in other related bugs were not approved.
(also, it looks like there's a stale "in-testsuite+" flag from when this originally landed - I'm toggling that to "?", since I assume the tests were backed out along with the code)
Flags: in-testsuite+ → in-testsuite?
This has had approval for most of a week - is it landing soon, or missing FF4?
Attachment #508507 - Flags: approval2.0+
Neil, please land this on mozilla-central if you want to get it in 2.2.
Whiteboard: [patchlove] → [patchlove][not-ready-for-cedar]
Ping for progress.
Status: REOPENED → RESOLVED
Closed: 10 years ago → 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla8
Flags: in-testsuite? → in-testsuite+
Component: XP Toolkit/Widgets: Menus → XUL
You need to log in before you can comment on or make changes to this bug.