397873 - Popup windows have menu bar when they used not to

Status: RESOLVED FIXED

(Toolkit :: UI Widgets, defect, P2)

Description

[Duncan Loveday]

User-Agent:       Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1; SV1; NGD_build; .NET CLR 1.1.4322; .NET CLR 2.0.50727)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007092605 Minefield/3.0a9pre

A popup window opened from javascript using "window.open" now seem to have a location and menu bar by default when they used not to and I have had no luck turning them off using "location=no", "toolbar=no", "menubar=no" etc.

Reproducible: Always

Steps to Reproduce:
1. Open the attached, click the button.
2.
3.
Actual Results:  
With the latest trunk, a popup window with location and menu bar is displayed.

Expected Results:  
The popup window should have no location or menubar.

Comment 1

[Duncan Loveday]

Attachment: Test HTML source

Comment 2

[Bob Clary [:bc] (inactive)]

Not, js eng as far as I can tell. Over to Core:DOM Level 0. Behavior changed since 20070831. Duncan, if you can go to <ftp://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/>, install nightly builds and find the exact date this changed it would help.

Comment 3

[Aiko]

It's intentional, see bug 337344.

Comment 4

[Bob Clary [:bc] (inactive)]

Alright, lets dupe it.

Comment 5

[Duncan Loveday]

Guys, sorry to reopen but bug 337344 only talks about the location bar.

There's also a change in behaviour of the menu bar as of 2007-09-18 that I don't think is covered by the other bug.

Comment 6

[Gabriel Chadwick]

Getting this to with Mozilla/5.0 (Windows; U; Windows NT 6.0; fr; rv:1.9a9pre) Gecko/2007100304 Minefield/3.0a9pre ID:2007100304 . Very annoying. What I understand of bug 337344 the goal is to add the location bar not the menu... I can't see any utility to that and it was not intended by bug 337344. Maybee related to bug 398507 ?

Confirm

Comment 7

[Ria Klaassen (not reading all bugmail)]

Sorry; should have said that a regression range test showed that this bug was caused by bug 127244. I pasted the bug number in the dependencies "blocking" field though, but that is clearly not enough.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Neil says:
"I think bug 397873 is caused because of a min-height override in skin; can you try making these rules !important http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/content/xul.css&rev=1.104&mark=269-270#265"

Comment 9

[[e-mail]]

Attachment: This seems to fix it

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 284790 [details] [diff] [review]
This seems to fix it

This isn't sufficient... I'll post a screenshot of what I see.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Attachment: screenshot with attachment 284790

Adding a "overflow-y: hidden" only fixes it partially... there's still an extra grey background.

Comment 12

[[e-mail]]

Attachment: Updated patch

My fault for using a slightly different testcase, sorry.

I guess this version might impose a Ts/Txul hit though...

Comment 14

[Daniel Veditz [:dveditz]]

duplicate 400000 claims this is a problem in full-screen mode (which Gavin says is also fixed by this patch). That aspect should be tested when this bug is verified.

Comment 16

[Alfred Kayser]

While you are at it, could you please change the 0% into 0 (or 0px). 
The 0% makes the CSS parser change the number into a floating point number (and even divides it by 100). Just 0 is enough. 

By the way, this all seems to be a very brute force approach to hide the menubar, but still enable the menu (when used with key shortcuts).

Comment 17

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #16)
> By the way, this all seems to be a very brute force approach to hide the
> menubar, but still enable the menu (when used with key shortcuts).

Yeah, I'm starting to wonder whether we just want to back out the patch for bug 127244... It's caused a few problems that weren't foreseen, and I'm not sure how likely it is that there will be more once this patch lands. Shipping with bug 127244 for another release wouldn't be the end of the world, I don't think. Neil, what do you think?

Comment 18

[[e-mail]]

(In reply to comment #16)
>While you are at it, could you please change the 0% into 0 (or 0px). 
>The 0% makes the CSS parser change the number into a floating point number (and
>even divides it by 100). Just 0 is enough. 
Unfortunately 0 is the default value here; 0% was chosen as a "magic" value.
(Sorry, but I forget the bug where 0% was made magic.)

(In reply to comment #17)
>It's caused a few problems that weren't foreseen
Only in Firefox, and then only because I forgot its toolbars work differently.

Comment 19

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 284852 [details] [diff] [review]
Updated patch

a- for M9 by endgame drivers

Comment 20

[Reed Loden [:reed]]

Comment on attachment 284852 [details] [diff] [review]
Updated patch

I just built a build with this patch, and it indeed fixes the problem.

Comment 21

[[e-mail]]

Fix checked in.

Comment 22

[Gabriel Chadwick]

Using Mozilla/5.0 (Windows; U; Windows NT 6.0; fr; rv:1.9b2pre) Gecko/2007111004 Minefield/3.0b2pre the menu as disappeared but the adress bar is not visible now. Sound like a regression ? Could anyone comment to explain.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Attachment: screenshot

The location bar appears for me using this bug's testcase and Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007111015 Minefield/3.0b2pre . I do see the menuitem border appear above the toolbar when I mouseover it, though (see screenshot).

Comment 24

[Gabriel Chadwick]

Attachment: Regression screenshot

Ok this cause a regession for me when the address bar and menu bar are on same UI line in my customized UI. Tested with a new profile and all. Screenshot as example.