Closed Bug 72747 Opened 23 years ago Closed 20 years ago

implement overflow-y and overflow-x properties

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: cosmic, Assigned: dbaron)

References

()

Details

(Keywords: css3, testcase, Whiteboard: WG[patch])

Attachments

(3 files, 5 obsolete files)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; 0.8.1) Gecko/20010319
BuildID:    2001031904

When assigning style="OVERFLOW-Y: scroll; HEIGHT:?px;" on a <div> it doesn't
automatically scroll the contents, they overflow outside the original div.

At
  http://www.uq.net.au/~zzcprows/Development/mozilla_issues.html
the text should scroll within the black border of the div and not overflow
outside as it does now.

Reproducible: Always
Steps to Reproduce:
1. Goto:  http://www.uq.net.au/~zzcprows/Development/mozilla_issues.html
2. Notice that the text overflows outside of the black border


Actual Results:  The text overflows outside of the black border.

Expected Results:  The text should scroll within the black border.
mozilla does not implement the overflow-y property, which is not yet a part of
any CSS standard.... are we planning to do this sometime?  changing this to an
rfe, but I am guessing this is invalid.
Assignee: jst → pierre
Severity: major → enhancement
Status: UNCONFIRMED → NEW
Component: DOM Style → Style System
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
Summary: style="OVERFLOW-Y: scroll; HEIGHT:?px;" on a <div> not scrolling → implement overflow-y and overflow-x properties
We have our own extensions to do this -- overflow: -moz-horizontal or something.

I suggest WONTFIX.
Whiteboard: WONTFIX ? -- non standards compliant
Our extensions to 'overflow' are -moz-scrollbars-none, -moz-scrollbars-
horizontal, and -moz-scrollbars-vertical.

I would close the bug as WontFix in its current description but I think that we 
should ask the WG to legitimize our extensions or propose a similar mechanism to 
handle the overflow.  Reassigned to Ian to promote the idea at the next meeting.
Then reassign to me for the implementation...
Assignee: pierre → ian
Target Milestone: --- → Future
Since I do not represent Netscape or Mozilla at the WGs, it is out of line to
ask me to promote an idea for Netscape or Mozilla. Reassigning to Daniel.

Note. I personally am agnostic on this issue; I would not be averse to seeing it
in the specs. I believe Microsoft would love it, so it should be easy to get in.
Assignee: ian → glazman
Keywords: css3
Whiteboard: WONTFIX ? -- non standards compliant → WG
imho overflow-x  and -y should be mapped to -moz-scrollbars-horizontal 
and -vertical.

because microsoft already uses the names, it is not probable that they 
would be defined in any other way. maybe it even becomes part of the 
spec one day. 

and because overflow-x and -y are already used on some sites for internet 
explorer, it would be an increase in real world compatibility. it would not 
break anything, at least.
I suggest WONTFIX, this isn't part of any standard and is not even in the
CSS3-Drafts. Currently it's only used to fix some bugs in IE.
This is in the working draft for CSS3
http://www.w3.org/TR/2002/WD-css3-box-20021024/#the-overflow-x

I would suggest that until the CSS3 spec is approved, there should be individual
attributes called: -moz-overflow-x and -moz-overflow-y, where the possible
values are visible, auto, scroll, and hidden.

That would allow the logic an usage to be put in place now, and then when the
CSS3 spec is approved, it can be implemented along their guidelines.
Note that the suggested CSS3 spec is not at all what IE implements and not what
we have either....  (eg overflow-x: scroll; overflow-y: auto;)
How does what IE does differ from the CSS3 proposal?
I was under the impression that the example in my comment did not work in IE...
that's the only explanation I can see for the:

overflow: scroll;
overflow-x: none;

thing I see on so many sites...

I could well be mistaken, of course.  Someone who actually has reliable access
to an IE that is not 5.0 (which is what I have) should test all 16 combinations.
Here's a link to the IE reference for overflow-x.  Obviously overflow-y is 
similar.

http://msdn.microsoft.com/library/default.asp?
url=/workshop/author/dhtml/reference/properties/overflowx.asp

It looks like the only difference that I notice is IE doesn't support "inherit" 
as a value.
*** Bug 204309 has been marked as a duplicate of this bug. ***
*** Bug 220289 has been marked as a duplicate of this bug. ***
Attached file basic test case
Keywords: testcase
Blocks: 42492
*** Bug 241902 has been marked as a duplicate of this bug. ***
Blocks: 72540
Attached patch CSS half of patch (untested) (obsolete) — Splinter Review
Assignee: daniel → dbaron
Status: NEW → ASSIGNED
Attached patch patch (untested) (obsolete) — Splinter Review
Attachment #156556 - Attachment is obsolete: true
Whiteboard: WG → WG[patch]
Though this doesn't test the 'auto' cases with x overflow and y overflow
separately...
Attached patch patch (obsolete) — Splinter Review
Attachment #156582 - Attachment is obsolete: true
Comment on attachment 156620 [details] [diff] [review]
patch

This patch does a bunch of things:

It does the normal stuff for adding a CSS property (without a -moz- prefix,
since IE already implements essentially the same property).  It puts perhaps
slightly more effort into correct serialization of the shorthand when possible
for backwards compatibility with when it wasn't a shorthand (although it never
produces -moz-scrollbars-vertical and -moz-scrollbars-horizontal).  It also
ensures that the computed values of overflow-x and overflow-y are always equal
when one of them is 'visible' or '-moz-hidden-unscrollable' (by moving away
from those values if one of the values is in that set and the other isn't).

It makes the changes needed to propagate two values through the viewport
overflow override.

It adds two convenience methods to nsStyleDisplay that simplify many of the
changes in layout.  Many of the other changes are simple because of the
invariant about mOverflowX and mOverflowY being equal if one of them is
...VISIBLE or ...CLIP.

It fixes the callers of the nsIScrollable API to actually use the enums on that
API (which are the only values that make sense) rather than NS_STYLE_OVERFLOW
values.

It moves ScrollbarStyles to nsPresContext and clearly documents the limitation
on the possible values there (more limited than previously documented), and
enforces that limitation.  (It's the NS_STYLE_OVERFLOW_* values that correspond
to the three values on the nsIScrollable API).

The current nsIScrollable API has "default" scrollbar preferences (persistent
across pages, which are used only for handling the scrolling attribute /
'overflow' property of frame/iframe elements) and "current" scrollbar
preferences (which are used only to suppress scrollframe creation on frameset
documents).  This patch removes the latter (and the reset method that sets the
latter to the former) and handles the suppression of scrolllframe creation on
framesets in a simpler way (a simple API on nsIHTMLDocument).  This also means
that the contents of an iframe with scrolling="no" have the same scroll frame
setup under the root frame as all other documents (I remember hitting bugs
caused by that difference in the past).  It likewise moves the testing of the
scrollbar preferences (now only "default") into nsGfxScrollFrame, which now
makes it easy to implement scrolling="yes" (and equivalents) for FRAME and
IFRAME (which we'd never implemented correctly before -- we treated it as
scrolling="auto").

This patch removes the GetScrollPreference API from nsIScrollableFrame and
makes the callers use GetScrollbarStyles instead (which is better for their
needs anyway).

This patch removes the SetScrollbarVisibility API from nsIScrollableFrame and
replaces it with a single line of CSS for the anonymous div inside non-textarea
text inputs (which was the only use).  Note that the internal variables for
that pref were also used for print preview, but the null-checks of the
scrollbar boxes should be sufficient.

This cleans up a bit of code in nsGfxScrollFrame that did more work than needed
when the overflow style was 'hidden'.

It cleans up overuse of |out| in nsIScrollable.idl .
Attachment #156620 - Flags: superreview?(roc)
Attachment #156620 - Flags: review?(roc)
Sounds great, I'll review it soon.

At first I wasn't convinced about making it overflow-x and overflow-y but I
think I've convinced myself that you're right.
Comment on attachment 156620 [details] [diff] [review]
patch

Actually, hold on.  I've regressed attachment 145385 [details].
Attachment #156620 - Flags: superreview?(roc)
Attachment #156620 - Flags: review?(roc)
Attached patch patchSplinter Review
Attachment #156620 - Attachment is obsolete: true
Attachment #156629 - Flags: superreview?(roc)
Attachment #156629 - Flags: review?(roc)
Comment on attachment 156629 [details] [diff] [review]
patch

Basically looks great. I'm not so confident about the style changes since my
style knowledge is fairly weak, and technically I'm not a style peer, but I
think you should just go ahead anyway.

See
http://lxr.mozilla.org/mozilla/source/layout/html/forms/src/nsListControlFrame.
cpp#1252
We can do that later.

+  // if we have 'auto' scrollbars or we dynamically changed to 'hidden'
+  // look at the vertical case
   if (styles.mVertical != NS_STYLE_OVERFLOW_SCROLL) {

I think this comment is incorrect. A dynamic change to 'hidden' would cause a
reframe. The only way styles.mVertical could be HIDDEN here is as a propagated
style from the viewport or imposed by the container preferences.

 [scriptable, uuid(61792520-82C2-11d3-AF76-00A024FFC08C)]
 interface nsIScrollable : nsISupports

Since you changed the interface I think it'd be best to give it a new UUID.
Attachment #156629 - Flags: superreview?(roc)
Attachment #156629 - Flags: superreview+
Attachment #156629 - Flags: review?(roc)
Attachment #156629 - Flags: review+
This broke DEBUG build:

g:/Warpzilla\mozilla\content\base\src\nsStyleContext.cpp(88) : warning C4554: '<
<' : check operator precedence for possible error; use parentheses to clarify pr
ecedence
g:/Warpzilla\mozilla\content\base\src\nsStyleContext.cpp(88) : warning C4554: '&
' : check operator precedence for possible error; use parentheses to clarify pre
cedence
g:/Warpzilla\mozilla\content\base\src\nsStyleContext.cpp(751) : error C2039: 'mO
verflow' : is not a member of 'nsStyleDisplay'
        g:\Warpzilla\mozilla\dist\include\content\nsStyleStruct.h(699) : see dec
laration of 'nsStyleDisplay'
../../../dist\include\xpcom\nsCOMPtr.h(228) : warning C4624: 'nsDerivedSafe<T>'
: destructor could not be generated because a base class destructor is inaccessi
ble
        with
        [
            T=nsPresContext
        ]
        g:/Warpzilla\mozilla\content\base\src\nsStyleContext.cpp(908) : see refe
rence to class template instantiation 'nsDerivedSafe<T>' being compiled
        with
        [
            T=nsPresContext
        ]

  fprintf(out, "<display data=\"%d %d %f %d %d %d %d %d %d %ld %ld %ld %ld %s\"
/>\n",
    (int)disp->mPosition,
    (int)disp->mDisplay,
    (float)disp->mOpacity,      
    (int)disp->mFloats,
    (int)disp->mBreakType,
    (int)disp->mBreakBefore,
    (int)disp->mBreakAfter,
    (int)disp->mOverflow,
    (int)disp->mClipFlags,
    (long)disp->mClip.x,
    (long)disp->mClip.y,
    (long)disp->mClip.width,
    (long)disp->mClip.height,
    URICString(disp->mBinding).get()
    );

mOverflowArea should go away
mOverflowX and mOverflowY should be used insted of mOverflow.
So, this means that we're going to support more cases of visibly unscrollable
scrollbars, is there an existing bug on the appearance of such scrollbars?
Bug 76197.
Checked in for a short time, but backed out (in 2 pieces) due to pageload time
regression.  I'll try relanding in more finely-grained pieces another time.
Looks like you may need to rev the UUIDs for nsIDOMCSS2Properties and
nsIScrollable (though you seemed to cover nsIScrollable in your checkin).
For the record, my standard set of testcases is bug 69355 comment 34.
I think part of the problem was the text input changes.
The frame / iframe changes also, to my surprise, seem to be a part of the
regression.
Everything is landed except for:
 * the text input changes and the removal of the nsIScrollableFrame API that
they used to suppress scrollbars
 * the change that makes the frame construction around the root more consistent
when in a frame with scrolling="no".

Both of these hurt pageload time, so I'm just skipping them.  I did modify the
latter so it checks both X and Y, though, and fix the comments around it.  I
also fixed up a few comments that I missed earlier:  mostly just changing
NS_STYLE_OVERFLOW_* to nsIScrollable::Scrollbar_*.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
Comment on attachment 156629 [details] [diff] [review]
patch

>+           /* Mozilla extensions */
>+           attribute DOMString        overflowX;
>+                                        // raises(DOMException) on setting
>+
>+           attribute DOMString        overflowY;
>+                                        // raises(DOMException) on setting
>+

Shouldn't that read 'MozOverflowX' and 'MozOverflowY'? Since it seems that the
properties are implemented with a -moz- prefix (quite logical, since css3-box
isn't in CR).

By the way, this should probably be mentioned on www-style or mailed directly
to Bert Bos, the editor of css3-box. Two UAs now support the properties so it
would not be very logical to extend 'overflow' instead.
Those properties were already co-opted by IE so we're not breaking the Web by 
using them again. Hence the lack of -moz- prefixes.

The fact that Mozilla implements this should not be an argument about whether to 
keep this feature in the drafts or not. That would be a very bad precedent (it 
would mean that all someone had to do to get a feature into CSS would be to get 
it implemented in two or more UAs).
In that case, shouldn't the following lines read a little different? (Line
breaks made after 'mOverflowX,' and 'mOverflowY' here should be ignored.)

>+CSS_PROP_DISPLAY(overflow-x, overflow_x, MozOverflowX, Display, mOverflowX,
> eCSSType_Value, PR_FALSE, kOverflowSubKTable)
>+CSS_PROP_DISPLAY(overflow-y, overflow_y, MozOverflowY, Display, mOverflowY,
> eCSSType_Value, PR_FALSE, kOverflowSubKTable)

That third paramter should be 'OverflowX' and 'OverflowY' if I'm not mistaken.
(Actually, I just compare with them other lines.)

Another question: how long will Mozilla have support for the '-moz-scrollbars-*'
values?
They do -- that patch didn't compile, since I missed one spot when removing the
Moz, and I removed the Moz long before checking in.
This fix caused bug 258228.
This patch appears to have caused bug 282750
You need to log in before you can comment on or make changes to this bug.