Chrome flags are never set on unparented XUL windows

VERIFIED FIXED in mozilla1.9.3a1



10 years ago
4 years ago


(Reporter: aakashd, Assigned: neil)


1.9.1 Branch
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)


(Whiteboard: [MozMill1.2Testday])


(3 attachments)



10 years ago
using the attached test case, the modal dialog API hangs and does not respond to the instancing of the clear recent history dialog via the tools menu.


10 years ago
Whiteboard: [MozMill1.2Testday]


10 years ago
Blocks: 509125
That CRH dialog is weird! I tried to get a fix for that but I have no idea where to start. Here some observations:

When I let go output from within the getDialog function go to the console I never see an entry for this window. I have three windows open main browser window, Mozmill window, and the Error Console. That means I get the following BrowserChrome flags: 4094, 2147487422, and 2147485198. There is no flag listed for the opened Clear Recent History window.

Comparing the opener to other instances of modal dialogs this one uses null for its parent window. But the modal attribute is set. So could it be that when null is specified the window is not listed by getXULWindowEnumerator("")?

As result we hang in our getDialog function:

Boris, do you know why this window is not listed by the enumerator?
Summary: Modal Dialog hangs on traversing the menu API to get to Clear Recent History → Modal dialog "Clear Recent History" is not listed by wm.getXULWindowEnumerator
I have no idea.  Neil might know...  In general, is the issue just that it's app-modal?
Ok, I have flipped null against aParentWindow in sanitize.js and everything works fine. The CRH dialog is now listed when retrieving an enumerator with getXULWindowEnumerator. I know nothing about how the window list is build but by using no parent window seems to create an independent window outside of the window list which will not be detected when the construction of the list happens.
Component: Mozmill → General
Product: Testing → Core
QA Contact: mozmill → general
Version: unspecified → 1.9.1 Branch
See also bug 309406 (which added the ifdef for Mac) and bug 464930 (CRH shouldn't be a modal dialog).

Comment 5

10 years ago
I don't know about the Mac (which is the only OS that uses a null parent for
the CPH dialog). I can't reproduce the problem on Windows.
OS: All → Mac OS X
Comment on attachment 393277 [details]

Forgive my ignorance ... but how do you run this test?
Steven, just clone the mozmill-test repository ( and save the test e.g. to firefox/helperClasses. Open Mozmill and run the test. You have to put it into the correct folder due to shared modules dependencies.

The same situation can be observed on Windows when changing the first parameter of the window opener to null. It would be helpful to have a better testcase here which doesn't rely on Mozmill. I'll try to create one.
Keywords: testcase-wanted
OS: Mac OS X → All
This XUL file checks all the open windows and displays their chromeFlags.

While writing this code I noticed that I was wrong. The window enumerator returns the correct window instances but opening the CRH with null as parent window seems to ignore the modal flag given to the openWindow function. The flag is not added to the windows chromeFlags but let the window be opened modal. That means we cannot evaluate if this window is modal or not.

Comment 9

10 years ago
Can you call nsIWebBrowserChrome::IsWindowModal perhaps?
(QI to nsIInterfaceRequestor and getInterface an nsIWebBrowserChrome)
Thanks Neil! In my testings on Friday I forgot the nsiInterfaceRequestor part so that function wasn't working for me. Now we get the correct modal state of the CRH window. I will incorporate the patch for Mozmill in bug 492136.

Does it mean the behavior of the underlying component is ok and will not be changed?
No longer blocks: 509125
As Neil mentioned on IRC chrome flags should only be used for content windows (pop-ups). So is there anything we can do here or should this bug be closed?
Summary: Modal dialog "Clear Recent History" is not listed by wm.getXULWindowEnumerator → Chrome flags for modal dialogs without a parent are ignored

Comment 12

10 years ago
When opening a window from another window (either modal or modeless) then the flags calculated by the window watcher are explicitly set on the newly created chrome window. However if the window is opened without a parent then no chrome flags are set, the XUL window's chromeFlags property has a default value and the window always opens with all of its chrome. Usually nobody cares since the only parentless windows are either the startup browser (which always has all of its chrome) or a random XUL window which doesn't care about chrome flags.
Summary: Chrome flags for modal dialogs without a parent are ignored → Chrome flags are never set on unparented XUL windows

Comment 13

10 years ago
The simpler patch would have been to add a call to SetChromeFlags to the one caller of CreateTopLevelWindow that doesn't do it itself, but since we're passing the chrome flags around anyway I thought I'd centralise it.
Assignee: nobody → neil
Attachment #393487 - Flags: review?(jst)
Attachment #393487 - Flags: review?(jst) → review+

Comment 14

10 years ago
Pushed changeset 5798118100e1 to mozilla-central.
Closed: 10 years ago
Resolution: --- → FIXED
Verified fixed with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.3a1pre) Gecko/20090821 Minefield/3.7a1pre ID:20090821030931

Neil, do you think it's worth to have this patch in 1.9.2?
Target Milestone: --- → mozilla1.9.3a1

Comment 16

10 years ago
Mozmill is the only consumer that I know of, so it's up to you, basically.
Comment on attachment 393487 [details] [diff] [review]
Proposed patch

At the moment our Mozmill tests don't use the flags anymore but it could happen that we have to base on them. So it would be fantastic to get it into 1.9.2.
Attachment #393487 - Flags: approval1.9.2?
I suspect that this caused bug 514920
Depends on: 514920

Comment 19

10 years ago
Comment on attachment 393487 [details] [diff] [review]
Proposed patch

Caused a regression, can't approve without a complete rollup.
Attachment #393487 - Flags: approval1.9.2? → approval1.9.2-


8 years ago
Depends on: 651659
You need to log in before you can comment on or make changes to this bug.