scrollbars=no ignored for window.open

VERIFIED FIXED in mozilla1.0.1

Status

()

P3
normal
VERIFIED FIXED
17 years ago
4 years ago

People

(Reporter: chris, Assigned: danm.moz)

Tracking

({regression, topembed+})

Trunk
mozilla1.0.1
regression, topembed+
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [ADT2 RTM] [DIGBug],custrtm-, [Verified Trunk], URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

17 years ago
Clicking on each of the thumbnails on this page will open a popup, all using the
same window.open script. The 1st, 2nd and last ones have content that is bigger
then the window and scrollbars are appearing. They SHOULDN'T as i've defined
scrollbars=no in the window parameters:

window.open(htmldoc,'photoWindow','width=700,height=525,status=no,menubar=no,scrollbars=no,resizable=no');
}

I searched and searched for a dupe, as i can't imagine this hasn't been reported
already, but didn't come up with anything more recent then a few bugs closed in
2000.

I get this behavior with the following nightlies on my machines:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:0.9.8+) Gecko/20020223
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.8+) Gecko/20020301

And the person who reported this to me just grabbed 0.9.9:
Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:0.9.9) Gecko/20020310
confirming, linux trunk 2002-03-11-08.
Assignee: asa → jst
Status: UNCONFIRMED → NEW
Component: Browser-General → DOM Level 0
Ever confirmed: true
QA Contact: doronr → desale

Comment 2

17 years ago
Confirming this on Windows2000 SP2 / Mozilla 0.9.9

Comment 3

17 years ago
Severity = MEDIUM [No Crash, Functional failure, Can Results in Cosmetic 
failure]
Visibility = MEDIUM [Could be lot of real world website usage, Gets one point of 
compatibility with other browsers since it works on other browsers. gets one 
more point on compliance with adopted techonology, that is JS] 

Priority = Visibility * Severity

Priority = p3

adding word "qawanted" because I'm setting this priority on available data & if 
someone feels otherwise then please investigate this more & feel free to change 
this priority.
Keywords: qawanted
Priority: -- → P3

Comment 4

17 years ago
this is regression. i just downloaded the 0.9.8 release and it still worked there.
Keywords: regression
Over to danm who has dealt with issues such as this one before.
Assignee: jst → danm

Comment 6

17 years ago
adding myself to cc ... and i'm already searching lxr to find this... any hints
welcome... i could imagine that i would make a patch if i find some time... =)

Comment 7

17 years ago
*** Bug 132085 has been marked as a duplicate of this bug. ***
*** Bug 133085 has been marked as a duplicate of this bug. ***
rossi: as for the hints... I'd try searching in
embedding/components/windowwatcher, which is responsible for opening windows.

But maybe the another obscure component is responsible for this :)

Comment 10

17 years ago
Adding DIGBug to status.
Whiteboard: DIGBug

Comment 11

17 years ago
we should not be regressing on basic issues such as this. =>nsbeta1, topembed
Keywords: nsbeta1, topembed

Updated

17 years ago
Keywords: nsbeta1, topembed → nsbeta1+, topembed+

Updated

17 years ago
Whiteboard: DIGBug → [DIGBug]

Comment 12

17 years ago
adding adt3 to this bug.
Whiteboard: [DIGBug] → [DIGBug][adt3]

Comment 13

17 years ago
This is because of the fix for bug 111524.  Basically, since we call
GetChromeFlags before ApplyChromeFlags, the scrollbars never get hidden in the
first place. We end up just checking their visibility in GetChromeFlags and
virtually unsetting scrollbars=no there.
*** Bug 143412 has been marked as a duplicate of this bug. ***

Comment 15

17 years ago
Heres an idea for how to fix this.
The problem is caused because of this code in nsContentTreeOwner.cpp (it was
added to fix bug 111524)
PRBool scrollbarVisibility = mXULWindow->GetContentScrollbarVisibility();
  if (scrollbarVisibility)
    mChromeFlags |= nsIWebBrowserChrome::CHROME_SCROLLBARS;
  else
    mChromeFlags &= ~nsIWebBrowserChrome::CHROME_SCROLLBARS;
My fix is to move this code out of GetChromeFlags and into a new function (say
UpdateScrollbarFlag) that will be called by whatever piece of code is used in
embedding to toggle scrollbar s on/off
That way the code above is only called when the scrollbar is changed programmaticly.
Then GetChromeFlags will do what its supposed to do and calling GetChromeFlags
before you call ApplyChromeFlags wont be a problem.

Or the other fix is just to call ApplyChromeFlags before you call GetChromeFlags

I dont know which of the 2 fixes is better though.

Comment 16

17 years ago
I'm sorry, but this is really annoyng and visible. 

www.kaspersky.com - even flash is affected.

Comment 17

17 years ago
I only see scrollbars when the content of an opened window has bigger
width/height as the window.

I've changed all popups of all websites after a revision in which Mozilla showed
this behavior. Therefore it's more Tech Evaneglism than a real bug/missing feature.
(Reporter)

Comment 18

17 years ago
The fact that mozilla doesn't understand this very basic javscript is ground for
evangelism??? If you start contacting existing sites because of /this/ bug and
asking them to fix this on /their/ end i think it would make the Mozilla project
 look very bad. I also don't think that we need the extra thousand evangelism
bugs that that would create.

This is a mozilla bug plain and simple. In the example URL I posted the content
is very intentionally bigger then the opened window dimensions. I have used the
window.open feature list to crop the page for presentation. This isn't the only
time scrollbars show up that are not wanted. This is basic functionality,
learned by anyone who has ever scripted anything, written about in countless
books and tutorials. It needs to work.

Comment 19

17 years ago
For some of us this is a very serious functional bug.  I've spent the past 19
months building a system for delivering rich client applications in a browser. 
It will go live within a month and works great except for one huge bug:  the
presence of scrollbars causes the GUI to fly all over the place, ruining the
whole application.  And I can't find any acceptable workaround.  I'm watching a
year and a half of work evaporate because of a Mozilla regression.  Anybody
wanting to deliver rich GUIs with Mozilla will meet the same fate.

I had faith that fixing such an obvious regression would be a priority, but RC2
is out and it doesn't look like this is going to get fixed for Moz 1.0.

I chose Moz as my platform because I believed in what you guys are doing.  But I
don't know what I'm going to have to write in my software demo to explain why
this suddenly works so badly.  

I demo'd this software for Mozilla folks last September and would be glad to
show it again to prove the severity of this bug.  Just say the word and I'm
hopping on a plane to Mountain View.  

 - thanks.
(Assignee)

Updated

17 years ago
Target Milestone: --- → mozilla1.0.1

Updated

17 years ago
Whiteboard: [DIGBug][adt3] → [DIGBug][adt3 RTM]

Comment 20

17 years ago
I've just spent hours double checking my syntax and trying diffrent variations
of scrollbars=X with window.open trying to figure out why scrollbars were
showing up with Mozilla. Finaly I realized it might be a bug and stumbled across
this entry. As a web developer this type of functionaly is crucial to our
control of presentation. We have several sites that use popout windows with
scrollbars disabled, including our website. 

http://www.silentplanet.com/

click on the 'Pleasure' link at the bottom of the page. This opens a new window
with flash content. Next choose a purple bubble [Dubious Science] then click on
'How it Started' just right of 'the cream soda crisis' graphic. As you can see,
the clipped text layer extends beyond the bottom of the page and scrollbars are
shown, even though scrollbar=0 is specified. As the text scrolls up, the size of
the vertical scrollbar changes but never goes away completly. This seriously
impacts the visual design of the page. 

If this bug makes it to the 30 million subscribers of AOL we will have to
completely rethink these sites and future development. Mozilla was my hope of
having a truly standards complient browser that we could create great content
with. This is a serious move in the wrong direction. 

I'm also the CIO of our company and was planning on moving everyone from
Netscape 4.7.x to Mozilla or Netscape 7. This bug will cause a large number of
complaints from our design staff and will force me to use IE instead. 

Updated

17 years ago
Blocks: 143047
Whiteboard: [DIGBug][adt3 RTM] → [DIGBug] [adt3 RTM]
Created attachment 85293 [details] [diff] [review]
Don't ask the chrome for scrollbar visibility before the chrome is loaded...

Danm, looks like this regressed at some point, and your name is all over this
:-)

Wanna review?

Updated

16 years ago
Whiteboard: [DIGBug] [adt3 RTM] → [DIGBug] [adt3 RTM],custrtm-

Comment 22

16 years ago
*** Bug 146422 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 23

16 years ago
Created attachment 85811 [details] [diff] [review]
don't store scrollbar visibility state in mChromeFlags

  The problem was that some random bit of code (nsBrowserContentListener) was
unfortunately asking for browser chrome flags before the window had been
initialized. At this point the content window did exist, but it didn't know yet
whether it should have scrollbars. Johnny's patch prevents
nsBrowserContentListener's untimely question from resetting member variable
mChromeFlags by noting that it's too early to ask.
  That works (thanks for finding that, jst!) but it's still subject to code
ordering quirks (though a lot less so). I'd like instead to go with this patch,
which avoids ordering quirks by never reflecting scrollbar visibility back into
mChromeFlags.
Attachment #85293 - Attachment is obsolete: true
Comment on attachment 85811 [details] [diff] [review]
don't store scrollbar visibility state in mChromeFlags

sr=jst
Attachment #85811 - Flags: superreview+
ADT, pelase reconsider this one, this fix is as safe as they get, and it fixes a
very basic and often annoying bug.
Whiteboard: [DIGBug] [adt3 RTM],custrtm- → [DIGBug],custrtm-
Comment on attachment 85811 [details] [diff] [review]
don't store scrollbar visibility state in mChromeFlags

r=bryner
Attachment #85811 - Flags: review+

Comment 27

16 years ago
r=jag fwiw ;-)
(Assignee)

Comment 28

16 years ago
.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 29

16 years ago
Is this really fixed or is the www.kaspersky.com appearing Flash window
scrollbars an other issue?

2002060504 trunk

Comment 30

16 years ago
it's not the same problem. They don't have any instance of 'scrollbars=no' 
and they docwrite the content into that window, where they give the window 
a size of 350x300, and try to fit a flash that is sized 350x300 into, without
consideration of whitespace and margins, etc. It's easy to write this 
compatibly; I don't consider it a bug.

Comment 31

16 years ago
Are the scrollbars on the popups on these sites due to this bug?

(warner bros) http://yayasisterhood.warnerbros.com/teaser.html

http://www.virginradio.co.uk/index.html

Gecko/20020605 branch

Comment 32

16 years ago
(Those popups don't have scrollbars in mozilla 0.9.4.1)

If this is a different regression please advise asap as I agree it is annoying 
and unprofessional looking.

Comment 33

16 years ago
looks like 6/4 trunk build fixes the YAYA site but not the Virgin Radio popup.
(Reporter)

Comment 34

16 years ago
Yes, the patch isn't on the 1.0 branch, but has been fixed for a few days on the
trunk.

Looking at the source for the virgin site it doesn't look like they are setting
the "scrollbars" parameter when they open the popup, they're just hoping it
fits. Apparently it doesn't so mozilla is shoeing scrollbars. That is not this
bug, and in fact most probably is an evangelism thing. 

Comment 35

16 years ago
my bad -->yes virgin is ____ in 0.9.4 also.

Evangelism is already in progress w/them. Thanks.

Comment 36

16 years ago
desale - can you pls verify this fix on the trunk, and check around to see if
there were any related regressions.
Keywords: adt1.0.1, approval, mozilla1.0.1
Whiteboard: [DIGBug],custrtm- → [ADT2 RTM] [DIGBug],custrtm-

Comment 37

16 years ago
i checked with the sites which i knew where it didn't work, and those are fine
now... didn't see any regressions, but i didn't look very hard either...

Comment 38

16 years ago
Created attachment 86720 [details]
Testcase Attached.

Attaching self explanatory testcase.

I tried this one with trunk builds (2002-06-04-o8-trunk) on WinNT & found that
this one is FIXED on trunk.

I tried this one with todays branch builds (2002-06-06-08-1.0.0) on WinXP &
found that this one is NOT FIXED on branch.

I also tried this one with another branch builds (2002-06-03-08-1.0.0) on Win2K
& found that this one is NOT FIXED on that branch either.

Comment 39

16 years ago
Not marking verified since it still exists on branch. Adding  [Verified Trunk]
on status whiteboard because its verified on trunk.
Whiteboard: [ADT2 RTM] [DIGBug],custrtm- → [ADT2 RTM] [DIGBug],custrtm-, [Verified Trunk]

Comment 40

16 years ago
adt1.0.1+ added. please get necessary drivers approval, and add fixed1.0.1
keyword once it lands in the branch.
Keywords: adt1.0.1 → adt1.0.1+

Comment 41

16 years ago
please land on the 1.0.1 branch. once there, remove the mozilla1.0.1+ keyword
and add the fixed1.0.1 keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+

Updated

16 years ago
Attachment #85811 - Flags: approval+
(Assignee)

Comment 42

16 years ago
patch is now also checked in to the 1.0/1.0.1 branch
Keywords: mozilla1.0.1+ → fixed1.0.1

Comment 43

16 years ago
*** Bug 154390 has been marked as a duplicate of this bug. ***

Comment 44

16 years ago
Verified on branch [2002-07-11] & Trunk [2002-07-12-08].
Status: RESOLVED → VERIFIED
Keywords: fixed1.0.1 → verified1.0.1
*** Bug 158555 has been marked as a duplicate of this bug. ***

Comment 46

16 years ago
*** Bug 159121 has been marked as a duplicate of this bug. ***

Comment 47

15 years ago
*** Bug 147416 has been marked as a duplicate of this bug. ***
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.