Closed Bug 130174 Opened 22 years ago Closed 22 years ago

scrollbars=no ignored for window.open

Categories

(Core :: DOM: Core & HTML, defect, P3)

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0.1

People

(Reporter: chris, Assigned: danm.moz)

References

(
URL
)

Details

(Keywords: regression, topembed+, Whiteboard: [ADT2 RTM] [DIGBug],custrtm-, [Verified Trunk])

Attachments

(2 files, 1 obsolete file)

don't store scrollbar visibility state in mChromeFlags
2.97 KB, patch
bryner
: review+
jst
: superreview+
jud
: approval+
Details | Diff | Splinter Review
Testcase Attached.
425 bytes, text/html
Details 
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
Confirming this on Windows2000 SP2 / Mozilla 0.9.9
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
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
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... =)
*** 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 :)
Adding DIGBug to status.
Whiteboard: DIGBug
we should not be regressing on basic issues such as this. =>nsbeta1, topembed
Keywords: nsbeta1, topembed
Keywords: nsbeta1, topembed → nsbeta1+, topembed+
Whiteboard: DIGBug → [DIGBug]
adding adt3 to this bug.
Whiteboard: [DIGBug] → [DIGBug][adt3]
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. ***
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.


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

www.kaspersky.com - even flash is affected.
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.

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.
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.
Target Milestone: --- → mozilla1.0.1
Whiteboard: [DIGBug][adt3] → [DIGBug][adt3 RTM]
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.
Blocks: 143047
Whiteboard: [DIGBug][adt3 RTM] → [DIGBug] [adt3 RTM]
Danm, looks like this regressed at some point, and your name is all over this
:-)

Wanna review?
Whiteboard: [DIGBug] [adt3 RTM] → [DIGBug] [adt3 RTM],custrtm-
*** Bug 146422 has been marked as a duplicate of this bug. ***
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+
r=jag fwiw ;-)
.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Is this really fixed or is the www.kaspersky.com appearing Flash window
scrollbars an other issue?

2002060504 trunk
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.
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
(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.
looks like 6/4 trunk build fixes the YAYA site but not the Virgin Radio popup.
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. 
my bad -->yes virgin is ____ in 0.9.4 also.

Evangelism is already in progress w/them. Thanks.
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-
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...
Attached file 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.
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]
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+
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+
Attachment #85811 - Flags: approval+
patch is now also checked in to the 1.0/1.0.1 branch
Keywords: mozilla1.0.1+ → fixed1.0.1
*** Bug 154390 has been marked as a duplicate of this bug. ***
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. ***
*** Bug 159121 has been marked as a duplicate of this bug. ***
*** 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.

Attachment

General

Creator:
Created:
Updated:
Size: