nsXULPopupManager::ShowPopup (or callers) should flush frames

RESOLVED FIXED

Status

()

RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Gavin, Assigned: enndeakin)

Tracking

({regression})

Trunk
regression
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 -
wanted1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

See bug 395314: trying to unhide and then open a popup doesn't currently work without forcing frame construction or using a setTimeout hack. nsXULPopupManager::ShowPopup should probably FlushPendingNotifications().
Blocks: 395314
Summary: nsXULPopupManager::ShowPopup callers should flush frames → nsXULPopupManager::ShowPopup (or callers) should flush frames
Or nsPopuBoxObject::ShowPopup should if the other caller of nsXULPopupManager::ShowPopup can't deal.
Blocks: 279703
Flags: blocking1.9?
Keywords: regression
(Assignee)

Comment 2

11 years ago
Sounds like nsXULPopupManager::GetFrameOfTypeForContent could just take a shouldFlush argument which flushes before retrieving the frame.
 
(Assignee)

Comment 3

11 years ago
Created attachment 283469 [details] [diff] [review]
flush the frames before showing a popup
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Attachment #283469 - Flags: superreview?(bzbarsky)
Attachment #283469 - Flags: review?(bzbarsky)
Flags: blocking1.9? → blocking1.9-
Whiteboard: [wanted-1.9]
Comment on attachment 283469 [details] [diff] [review]
flush the frames before showing a popup

A flush can run arbitrary script.  In particular, the presshell can be dead here after the flush...

I assume all consumers are OK with random script running here, right?
Attachment #283469 - Flags: superreview?(bzbarsky)
Attachment #283469 - Flags: superreview-
Attachment #283469 - Flags: review?(bzbarsky)
Attachment #283469 - Flags: review-
(Assignee)

Comment 5

11 years ago
(In reply to comment #4)
> (From update of attachment 283469 [details] [diff] [review])
> A flush can run arbitrary script.  In particular, the presshell can be dead
> here after the flush...

Ah, yes. So it should be ok to keep a strong reference to the presshell right?

> I assume all consumers are OK with random script running here, right?

Yes, the callers are expecting synchronous event handling so scripts will already be exected to run.

> Ah, yes. So it should be ok to keep a strong reference to the presshell right?

Yeah, I think that should be enough.
(Assignee)

Comment 7

11 years ago
Created attachment 283761 [details] [diff] [review]
keep a strong reference to the presshell
Attachment #283469 - Attachment is obsolete: true
Attachment #283761 - Flags: superreview?(bzbarsky)
Attachment #283761 - Flags: review?(bzbarsky)
Comment on attachment 283761 [details] [diff] [review]
keep a strong reference to the presshell

r+sr=bzbarsky
Attachment #283761 - Flags: superreview?(bzbarsky)
Attachment #283761 - Flags: superreview+
Attachment #283761 - Flags: review?(bzbarsky)
Attachment #283761 - Flags: review+
(Assignee)

Updated

11 years ago
Attachment #283761 - Flags: approval1.9?
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Flags: wanted1.9+
Whiteboard: [wanted-1.9]

Updated

11 years ago
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: xptoolkit.xul → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.