The default bug view has changed. See this FAQ.

XUL menupopups in HTML document disappear in Firefox 2.0.0.8

RESOLVED FIXED

Status

()

Core
XUL
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Marcus Better, Assigned: smaug)

Tracking

({fixed1.8.1.9, regression, verified1.8.1.10})

1.8 Branch
fixed1.8.1.9, regression, verified1.8.1.10
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.8.1.9 +
blocking1.8.1.10 +
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 4 obsolete attachments)

(Reporter)

Description

10 years ago
User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.5; Linux) KHTML/3.5.7 (like Gecko) (Debian)
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.8) Gecko/20071004 Iceweasel/2.0.0.8 (Debian-2.0.0.8-1)

I have an HTML web app that uses some XUL elements to provide pop-up menus. This worked fine in Firefox before 2.0.0.8. In version 2.0.0.8 the pop-ups  disappear very quickly after being shown. A workaround is to apply a small delay with setTimeout before showPopup.


Reproducible: Always
(Reporter)

Comment 1

10 years ago
Created attachment 286021 [details]
Test case

This shows a popup menu with the text "POPUP!". When run in Firefox 2.0.0.8 you can see that the popup only flashes very quickly and then disappears. If you comment out the call to elt.showPopup() and uncomment the setTimeout line instead, the popup stays on screen.

The test case is not perfect since it doesn't work in earlier versions. On 2.0.0.7 or 2.0.0.6 it gives an error: "elt.showPopup is not a function". However my full webapp does work on earlier versions and creates popups in much the same way.
This was repaired on trunk but it regressed again. Regression range for this is  http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&date=explicit&mindate=1186421100&maxdate=1186423079 so it is probably caused by Bug 355789.
Blocks: 355789
Keywords: regression
OS: Linux → All
Status: UNCONFIRMED → NEW
Ever confirmed: true
Marcus, thanks for the testcase. Is there any chance we can test the webapp in question too?

This seems to have regressed on branch between 2007-10-03 and 2007-10-04:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-10-03+03&maxdate=2007-10-04+05&cvsroot=%2Fcvsroot
Regression from bug 398404?

I suspect a relation with bug 400744 too.
The patch for bug 355789 wasn't checked in to branch (which 2.0.0.8 was built
from). Does this problem occur on trunk?
It is worksforme, using current trunk build, but I'm using the win2000 theme here.
However, I see a scrollbar appearing in the document, for a brief while, on trunk.
Created attachment 286077 [details]
Screenshot

This is what I see on trunk before I give another window focus. Then it disappears entirely.
(In reply to comment #6)
> This is what I see on trunk before I give another window focus. Then it
> disappears entirely.

Ok, that's what I can see too. I filed bug 401027 as a trunk bug.

This bug is now meant as the regression that can be seen on branch.
Version: unspecified → 1.8 Branch
No longer blocks: 355789
I tried effectively backing out bug 398404, and I still see this problem on branch.
So the menu comes down with the following stack:

#0  nsMenuPopupFrame::HideChain (this=0x8dceaa4)
    at ../../../../../mozilla/layout/xul/base/src/nsMenuPopupFrame.cpp:1855
#1  0xb55b0229 in nsMenuDismissalListener::Rollup (this=0x8dd8528)
    at ../../../../../mozilla/layout/xul/base/src/nsMenuDismissalListener.cpp:110
#2  0xb67ab9ca in nsWindow::Destroy (this=0x8b14d70)
    at ../../../../mozilla/widget/src/gtk2/nsWindow.cpp:391
#3  0xb57d6c3b in ~nsView (this=0x8b14d18) at ../../../mozilla/view/src/nsView.cpp:266
#4  0xb57d6e08 in nsIView::Destroy (this=0x8b14d18)
    at ../../../mozilla/view/src/nsView.cpp:304
...
#12 0xb53747c2 in nsFrameManager::RemoveFrame (this=0x8d75dcc, aParentFrame=0x8dca4f8, 
    aListName=0x0, aOldFrame=0x8dceaa4)
    at ../../../mozilla/layout/base/nsFrameManager.cpp:717
#13 0xb5332597 in nsCSSFrameConstructor::ContentRemoved (this=0x8da49e8, 
    aContainer=0x8c07e90, aChild=0x8b90c28, aIndexInContainer=0, aInReinsertContent=0)
...
#18 0xb533aa7f in nsCSSFrameConstructor::RestyleEvent::HandleEvent (this=0x8b13748)
    at ../../../mozilla/layout/base/nsCSSFrameConstructor.cpp:14286

I have no idea what this restyle is and why it's apparently happening later than it used to on branch.
I also backed out bug 393770 and I still see the problem.
At this point I start to wonder about the range...  Someone should really try to do a pull by date and see what happens.
I'm terribly sorry!
I had my Firefox2 build described as 2007-10-04, but it turns out it is from 2007-10-06.
I'll narrow the regression range again from there.
Again, sorry for this.
Ok, the regression on branch is between 2007-10-04 and 2007-10-05:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2007-10-04+03&maxdate=2007-10-05+05&cvsroot=%2Fcvsroot
which makes bug 384105 to be a likely candidate for the regression.
Ah, yes.  That I would buy.
Backing out bug 384105 fixes this.

Are we perhaps ungenerating after the (following?) open has happened because of the event thing?
Blocks: 384105
Flags: blocking1.8.1.9?
Flags: blocking1.8.1.10?
Assignee: nobody → Olli.Pettay
(Reporter)

Comment 16

10 years ago
(In reply to comment #3)
> Marcus, thanks for the testcase. Is there any chance we can test the webapp
> in question too?

Sure, send me a private mail if you still need it.
(Assignee)

Comment 17

10 years ago
Created attachment 286135 [details] [diff] [review]
possible patch

This is not perfect, but I don't know what else could be done.
If we recreate the frame for menu, don't unset menugenerated.
Attachment #286135 - Flags: review?(enndeakin)
(Assignee)

Comment 18

10 years ago
Created attachment 286137 [details] [diff] [review]
this one
Attachment #286135 - Attachment is obsolete: true
Attachment #286137 - Flags: review?(enndeakin)
Attachment #286135 - Flags: review?(enndeakin)
(Assignee)

Comment 19

10 years ago
Comment on attachment 286137 [details] [diff] [review]
this one

Neils, what do you think about this.
Attachment #286137 - Flags: review?(neil)

Comment 20

10 years ago
Comment on attachment 286137 [details] [diff] [review]
this one


>+    : mMenu(aMenu), mContent(aContent)

Maybe mContent(aContent) should be mPopup(aPopup) so it is clearer what node it refers to.

>+    if (mMenu) {

Could just null check this before creating the event, or just assert since this shouldn't be null.

Otherwise, looks OK.
Attachment #286137 - Flags: review?(enndeakin) → review+
(Assignee)

Comment 21

10 years ago
Created attachment 286141 [details] [diff] [review]
v3

I'm asking extra review in hope to prevent regressions.
Attachment #286137 - Attachment is obsolete: true
Attachment #286141 - Flags: superreview?(roc)
Attachment #286141 - Flags: review?(neil)
Attachment #286137 - Flags: review?(neil)

Comment 22

10 years ago
Comment on attachment 286141 [details] [diff] [review]
v3

I was trying to work out which frame it was better to check for but I think checking for the menu frame is OK.

As for the null-check, child is (surprise) a child of mContent so it's going to be null when mContent is ;-)
Attachment #286141 - Flags: review?(neil) → review+

Updated

10 years ago
Blocks: 400744
(Assignee)

Comment 23

10 years ago
Created attachment 286156 [details] [diff] [review]
v4, check also that mPopup hasn't been moved
Attachment #286141 - Attachment is obsolete: true
Attachment #286156 - Flags: superreview?(roc)
Attachment #286141 - Flags: superreview?(roc)
(Assignee)

Comment 24

10 years ago
Comment on attachment 286156 [details] [diff] [review]
v4, check also that mPopup hasn't been moved

>+          new nsASyncUngenerate(menu, child);

s/menu/GetContent()
(Assignee)

Comment 25

10 years ago
Created attachment 286157 [details] [diff] [review]
v5, fix the "typo" in previous patch
Attachment #286156 - Attachment is obsolete: true
Attachment #286157 - Flags: superreview?(roc)
Attachment #286156 - Flags: superreview?(roc)

Comment 26

10 years ago
Smaug/Bz any comments on risk level of this patch - we are about ready to start a 2009 and are not sure if we should take this.  Also - does this only affect XUL elements?
(Assignee)

Comment 27

10 years ago
This is "only" about xul popups/menus behaving badly.
Not sure about bug 400744, though not sure if the patch fixes that either.
Risk should be low, but not minimal. The risk is there in cases where
xul popups/menus are used in some not-so-usual way.
If this patch fixes bug 400744 I think we should take it for 2.0.0.9.

If it doesn't, then I'm a little torn.  If popups in XUL documents are not affected (something that needs to be checked), then I'm not sure XUL is used enough in HTML (or supported enough in that configuration) to put this in the firedrill release.
The testcase in bug 400744 appears to be a XUL dialog, and popups there were affected by this bug and fixed by this patch.

Comment 30

10 years ago
So my $.02 is since there is a workaround, this deals with XUL popups only we should put this in 2.0.0.10 when we have more time to test it...

Comment on attachment 286157 [details] [diff] [review]
v5, fix the "typo" in previous patch

sr=dveditz

approved for 1.8.1.9/1.8.1.10, a=dveditz for release-drivers
Attachment #286157 - Flags: superreview?(roc)
Attachment #286157 - Flags: superreview+
Attachment #286157 - Flags: approval1.8.1.9+
Attachment #286157 - Flags: approval1.8.1.10+
(Assignee)

Comment 32

10 years ago
Checked in.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
Added appropriate fixed keywords for branch release tracking.
Keywords: fixed1.8.1.10, fixed1.8.1.9
Flags: blocking1.8.1.9?
Flags: blocking1.8.1.9+
Flags: blocking1.8.1.11?
Flags: blocking1.8.1.10+
verified fixed 1.8.1.10 using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.8.1.10pre) Gecko/2007111503 BonEcho/2.0.0.10pre and the testcase from this bug.

Testcase is working fine now - adding verified keyword
Keywords: fixed1.8.1.10 → verified1.8.1.10

Updated

9 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.