As a security precaution, we have turned on the setting "Require API key authentication for API requests" for everyone. If this has broken something, please contact bugzilla-admin@mozilla.org
Last Comment Bug 524545 - A popup opened with large margins appears offscreen ("ASSERTION: Popup is offscreen")
: A popup opened with large margins appears offscreen ("ASSERTION: Popup is off...
Status: RESOLVED FIXED
[patchlove][not-ready-for-cedar]
: assertion
Product: Core
Classification: Components
Component: XP Toolkit/Widgets: Menus (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla8
Assigned To: Neil Deakin
:
:
Mentors:
Depends on: 841585 672912
Blocks: 626563 630684
  Show dependency treegraph
 
Reported: 2009-10-26 13:50 PDT by Neil Deakin
Modified: 2014-11-10 04:47 PST (History)
12 users (show)
enndeakin: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
testcase (407 bytes, application/vnd.mozilla.xul+xml)
2009-10-26 13:50 PDT, Neil Deakin
no flags Details
fix (13.88 KB, patch)
2009-10-28 07:44 PDT, Neil Deakin
roc: review+
Details | Diff | Splinter Review
Updated patch (12.82 KB, patch)
2010-12-12 11:12 PST, Neil Deakin
no flags Details | Diff | Splinter Review
fix the test (8.62 KB, patch)
2011-01-31 12:55 PST, Neil Deakin
no flags Details | Diff | Splinter Review

Description User image Neil Deakin 2009-10-26 13:50:26 PDT
Created attachment 408462 [details]
testcase

This causes an assertion and the popup isn't visible because it is offscreen.
Comment 1 User image Neil Deakin 2009-10-28 07:44:48 PDT
Created attachment 408843 [details] [diff] [review]
fix

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.
Comment 3 User image Daniel Holbert [:dholbert] 2009-11-03 14:04:48 PST
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... :)
Comment 4 User image Daniel Holbert [:dholbert] 2009-11-03 14:05:57 PST
(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
Comment 5 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2009-11-09 16:57:27 PST
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 6 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2009-11-09 16:59:56 PST
Done:
http://hg.mozilla.org/mozilla-central/rev/86a93c8c6a87
Comment 7 User image Neil Deakin 2010-12-12 11:12:41 PST
Created attachment 497168 [details] [diff] [review]
Updated patch
Comment 8 User image Serge Gautherie (:sgautherie) 2011-01-18 10:09:32 PST
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/
Comment 9 User image Serge Gautherie (:sgautherie) 2011-01-24 22:46:42 PST
Neil, what is the next step here?
Comment 10 User image Serge Gautherie (:sgautherie) 2011-01-26 17:51:11 PST
"blocking2.0=?":
Bug 616607 was blocking2.0+.
This is "blocking" work on bug 626563 that causes a perma-orange on SeaMonkey.
Comment 11 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-01-27 08:38:17 PST
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)
Comment 12 User image Neil Deakin 2011-01-31 12:55:18 PST
Created attachment 508507 [details] [diff] [review]
fix the test

This patch just adjusts the offsets in the test a bit. It doesn't change any non-test code from the previously reviewed patch.
Comment 13 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-02-23 10:18:47 PST
This should land; I'm happy to preserve the approval here. Can we get 'er done?
Comment 14 User image :Ehsan Akhgari 2011-02-23 16:23:17 PST
It's not clear which attachment should be landed here.  Please generate a single patch for landing, and mark everything else as obsolete.
Comment 15 User image Neil Deakin 2011-02-23 18:28:19 PST
None of it should be. Patches in other related bugs were not approved.
Comment 16 User image Daniel Holbert [:dholbert] 2011-02-23 18:56:36 PST
(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)
Comment 17 User image Johnathan Nightingale [:johnath] 2011-02-28 12:24:20 PST
This has had approval for most of a week - is it landing soon, or missing FF4?
Comment 18 User image :Ehsan Akhgari 2011-03-28 14:20:19 PDT
Neil, please land this on mozilla-central if you want to get it in 2.2.
Comment 19 User image Serge Gautherie (:sgautherie) 2011-04-23 10:55:24 PDT
Ping for progress.
Comment 20 User image Marco Bonardo [::mak] 2011-08-06 02:55:26 PDT
http://hg.mozilla.org/mozilla-central/rev/d0f042751940

Note You need to log in before you can comment on or make changes to this bug.