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)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: cosmic, Assigned: dbaron)
References
()
Details
(Keywords: css3, testcase, Whiteboard: WG[patch])
Attachments
(3 files, 5 obsolete files)
747 bytes,
text/html
|
Details | |
6.13 KB,
text/html; charset=UTF-8
|
Details | |
175.38 KB,
patch
|
roc
:
review+
roc
:
superreview+
|
Details | Diff | Splinter Review |
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.
Comment 1•23 years ago
|
||
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
Comment 2•23 years ago
|
||
We have our own extensions to do this -- overflow: -moz-horizontal or something. I suggest WONTFIX.
Whiteboard: WONTFIX ? -- non standards compliant
Comment 3•23 years ago
|
||
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
Comment 4•23 years ago
|
||
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.
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.
Comment 6•22 years ago
|
||
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.
Comment 7•22 years ago
|
||
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.
Comment 8•22 years ago
|
||
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;)
Comment 9•22 years ago
|
||
How does what IE does differ from the CSS3 proposal?
Comment 10•22 years ago
|
||
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.
Comment 11•22 years ago
|
||
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.
Comment 12•21 years ago
|
||
*** Bug 204309 has been marked as a duplicate of this bug. ***
Comment 13•21 years ago
|
||
*** Bug 220289 has been marked as a duplicate of this bug. ***
Comment 14•21 years ago
|
||
Comment 15•20 years ago
|
||
*** Bug 241902 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 16•20 years ago
|
||
Assignee: daniel → dbaron
Status: NEW → ASSIGNED
Assignee | ||
Comment 17•20 years ago
|
||
Attachment #156556 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
Whiteboard: WG → WG[patch]
Assignee | ||
Comment 18•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #156572 -
Attachment is obsolete: true
Assignee | ||
Comment 19•20 years ago
|
||
Though this doesn't test the 'auto' cases with x overflow and y overflow separately...
Assignee | ||
Comment 20•20 years ago
|
||
Attachment #156582 -
Attachment is obsolete: true
Assignee | ||
Comment 21•20 years ago
|
||
Assignee | ||
Updated•20 years ago
|
Attachment #156619 -
Attachment is obsolete: true
Assignee | ||
Comment 22•20 years ago
|
||
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.
Assignee | ||
Comment 24•20 years ago
|
||
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)
Assignee | ||
Comment 25•20 years ago
|
||
Attachment #156620 -
Attachment is obsolete: true
Assignee | ||
Updated•20 years ago
|
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+
Comment 27•20 years ago
|
||
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
Comment 28•20 years ago
|
||
mOverflowX and mOverflowY should be used insted of mOverflow.
Comment 29•20 years ago
|
||
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?
Comment 30•20 years ago
|
||
Bug 76197.
Assignee | ||
Comment 31•20 years ago
|
||
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.
Comment 32•20 years ago
|
||
Looks like you may need to rev the UUIDs for nsIDOMCSS2Properties and nsIScrollable (though you seemed to cover nsIScrollable in your checkin).
Assignee | ||
Comment 33•20 years ago
|
||
For the record, my standard set of testcases is bug 69355 comment 34.
Assignee | ||
Comment 34•20 years ago
|
||
I think part of the problem was the text input changes.
Assignee | ||
Comment 35•20 years ago
|
||
The frame / iframe changes also, to my surprise, seem to be a part of the regression.
Assignee | ||
Comment 36•20 years ago
|
||
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 37•20 years ago
|
||
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.
Comment 38•20 years ago
|
||
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).
Comment 39•20 years ago
|
||
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?
Assignee | ||
Comment 40•20 years ago
|
||
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.
Comment 41•20 years ago
|
||
This fix caused bug 258228.
Assignee | ||
Comment 42•20 years ago
|
||
This caused regression bug 258300.
Comment 43•19 years ago
|
||
This patch appears to have caused bug 282750
You need to log in
before you can comment on or make changes to this bug.
Description
•