Closed
Bug 503720
Opened 15 years ago
Closed 12 years ago
Implement vw/vh/vmin/vmax (viewport sizes) from CSS 3 Values and Units
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: ayg, Assigned: seth)
References
(Depends on 1 open bug, Blocks 1 open bug, )
Details
(Keywords: dev-doc-complete)
Attachments
(2 files, 6 obsolete files)
2.82 KB,
application/xhtml+xml
|
Details | |
15.59 KB,
patch
|
seth
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US) AppleWebKit/531.3 (KHTML, like Gecko) Chrome/3.0.193.0 Safari/531.3
Build Identifier:
CSS 3 Values and Units specifies vw, vh, and vm units. These are respectively proportional to the width of the viewport, the height of the viewport, and the minimum of the two. These would be extremely useful for authors -- quite a few very nasty hacks could be retired if we could use these (like <http://www.alistapart.com/articles/fauxcolumns/> and <http://matthewjamestaylor.com/blog/equal-height-columns-cross-browser-css-no-hacks>).
As far as I know, no other rendering engine implements any of these units yet. I've filed a bug against WebKit too:
https://bugs.webkit.org/show_bug.cgi?id=27160
See also this discussion on www-style:
http://lists.w3.org/Archives/Public/www-style/2009Jul/0021.html
Bug 472195 (adding rem element) might be useful for reference. On www-style, David Baron mentions an additional complication:
> What you're missing is that vh, vw, and vm require a new mechanism
> for handling dynamic changes (to the size of the viewport).
> (Percentages are different since they're relative to the parent or
> to the containing block.)
Reproducible: Always
Only if all the elements between the element it's on and the root element have display:block and width:100% (or width:auto and float:none and position:static or relative) and no margin, border, or padding. (Or something roughly like that.)
Reporter | ||
Comment 3•15 years ago
|
||
Also, 100vw is the full width of the viewport, not 1vw.
Updated•14 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 4•14 years ago
|
||
This is essential for feature parity with Flash text that resizes to fill the Flash view without using JavaScript + the resize event.
I think I will need this to hide an element just outside of the viewport in the same place, without relying on a specific screen size.
Comment 6•13 years ago
|
||
The current practice for that is to offset by 999999px.
Thanks for the tip. However, this is for a slide-in CSS-animation where the speed will rely on how far off from the viewport the element is located. To control the speed, it's location would have to be just outside the viewport.
Also, according to Wikipedia (http://en.wikipedia.org/wiki/Comparison_of_layout_engines_%28Cascading_Style_Sheets%29) this should be parity-IE9. I can't confirm it with my OS though.
Comment 8•13 years ago
|
||
(In reply to comment #7)
> Also, according to Wikipedia
> (http://en.wikipedia.org/wiki/
> Comparison_of_layout_engines_%28Cascading_Style_Sheets%29) this should be
> parity-IE9. I can't confirm it with my OS though.
Based upon my testing, WIE9 and WIE10PP1 both recognize |vw| and |vh| as valid units, but the units don't behave as described by the spec [1]; 100vw and 100vh end up being substantially wider and taller than the viewport.
[1A] http://dev.w3.org/csswg/css3-values/#the-vw-unit
[1B] http://dev.w3.org/csswg/css3-values/#the-vh-unit
Updated•13 years ago
|
Attachment #538870 -
Attachment mime type: application/octet-stream → application/xhtml+xml
I've been working with the win8 dev preview and I'm waiting to see support for the new units in a real (non-ie) browser. If I had the foggiest idea how I would contribute support myself, but sadly I don't. Is anyone working on this problem?
Comment 10•13 years ago
|
||
I'm not aware that anyone is, although I'm not the most informed person on layout happenings these days. I've thought about attempting to pick this up myself, as a way to learn more about the layout engine (in which I have lightly dabbled, mostly with painting-only changes, nothing affecting dimensions or positioning yet). But I have other projects I've lightly committed myself to first (bug 483446 for one), so this would be back-burner of back-burner for me at best.
When you say "If I had the foggiest idea how I would contribute support myself": are you saying you have no idea because you don't know the code, because you don't know C++, because you don't program, because you've never worked with Mozilla code before, because you're solely an interested observer, or what? If you know C++ but don't know Gecko, someone could help you get started and probably point you at the relevant code that would need touching. I'm willing to do that, to the extent that I can (part of the appeal of doing this myself would be that it's a little bit of a stretch, so I'd probably point you at others assuming you met questions I couldn't answer at some point), if you're interested.
Of course, if you're not interested, that's fine too -- everyone has their thing. But in case you are interested, but are just blocked on getting started and figuring out what to fix, we could help you with that -- just say the word.
Comment 11•13 years ago
|
||
I don't have the foggiest idea in that I don't know C++. I'm a professional developer, but I work with html/css/js. vm/vh/vw is a great solution to a few problems that have been plaguing me for some time, and I'd love to see it happen but I just don't have the skills to do it. So far I've been trying to find a way to create a javascript polyfill (more of my area) to bridge the gap until real support can be achieved, but I haven't a whole lot of luck there (at least nothing that's better than the ugly hacks I'm trying to replace).
Comment 12•13 years ago
|
||
Ah. There are core Gecko hackers who picked up C++ while solely working on Gecko itself, but I'd guess they're not too common. It's certainly doable, especially if you read a book on C++ as well (this is to an approximation what I did), but it's probably beyond Mozilla developers helping you with, unless you were to start that process yourself (and likely not on this exact bug).
And yeah, I think vw/vh/vm would be awesome too. My "problem" is that I work primarily on the JS engine (and try to dabble heavily elsewhere), so layout hacking and other non-JS work is merely the occasional side project.
If C++ isn't your thing, I suppose there's always the possibility you could contract out the work to someone. Although given Gecko's complexity, that's probably not too cost-effective for one random person, alas. Beyond that I'm not sure there's much you personally could do, although I might just be missing something.
Comment 13•13 years ago
|
||
Actually those units are extremely handy. specially vh wit calc(), so you can vertically center an element on a page... I hope to see this soon!
Blocks: css-values-3
Updated•13 years ago
|
Keywords: dev-doc-needed
Comment 14•12 years ago
|
||
Update on implementation: these units have been implemented in Chrome (not sure since which release) and Safari 6 Beta.
Comment 15•12 years ago
|
||
So. Yeah. This now works in Safari stable, Chromium stable and Internet Explorer 10.
And is certainly quite useful.
I hope you guys toss this in soon. Would it really be that hard? I have no idea what you are doing in the backend, but I vaguely recall a discussion of some master unit (twip? something like that) surely a new unit is just a question of hooking it up. and you have other viewport dependent stuff already like percentage which is often viewport dependent.
http://m8y.org/tmp/testcase272.xhtml <- this btw is a test of something that's moderately annoying in Webkit and IE.
I'm not sure if it is a spec prob or a premature optimisation.
Comment 16•12 years ago
|
||
Percentage isn't viewport-dependent, it's containing block-dependent. (...in general, at least. I think.) The constraint propagation system for percentages is pretty different from what would be needed for viewport-dependent units; I don't believe any such system exists in the layout engine right now.
I suspect this wouldn't be that difficult to hook up, tho (doing so efficiently *might* be another matter maybe), but I could be wrong about that. And I've been too busy to spend my copious spare time noodling on a fix here (as a way to learn more about the layout code), alas.
The first question is whether to implement these in the style system (like 'em' units) or in layout (like '%'). I think it will be simpler to implement them in the style system. I'm curious if others agree.
This means that:
* v* units get computed in the CalcLengthWith function in nsRuleNode.cpp
* (not necessary, but makes things a lot simpler) anything with v* units gets canStoreInRuleTree = false
* any style context with v* units gets a bit set on it, and if it's the root-most style context with the bit, gets put in a list of such style contexts on the style set (and removed from that list when destroyed)
* if the list is non-empty, then when the viewport height or width changes, we walk the frame tree looking for frames with such root-most style contexts with the bit, and we reresolve their subtrees. (This takes care of inheritance, and also takes care of any descendants that independently use v* units.)
This approach could be optimized further (in particular, by maintaining a list of frames with such style contexts), but I think that's probably sufficient for a first pass.
Comment 18•12 years ago
|
||
Implementing in the style system would have the distinct benefit of them automatically working everywhere. So it definitely seems preferable a priori; I agree that it's probably simpler to implement in addition to that.
Comment 19•12 years ago
|
||
WRT comment #16 by Jeff.
In my defense I know percentage is containing block - I just meant that the containing block can be the viewport, and my understanding was that you guys had generic layout units so you'd think this would just be one more syntax for something you have to calculate anyway. From the other comments it does sound like there's two ways to do it.
Obviously the more generic one would be better. Would want to use vw/vh absolutely everywhere.
I do hope you don't end up with that silly Webkit/IE bug where if the window is resize, the width/height of blocks changes, but not the text size, until I do a refresh or delete/add the rule.
Javascript hacks are so annoying.
(at least, I assume it is a bug they both share - I guess there's some chance the spec was badly written or something)
Comment 20•12 years ago
|
||
> I just meant that the containing block can be the viewport
Yes, but when it is the thing is actually a child of the viewport, not in an arbitrary place in the box tree. So if the viewport changes size, it reflows its kids an everything works. To do vh, when the viewport changes size you have to find all the subtrees that depend on that and reflow them.
Summary: Implement vw/vh/vm (viewport sizes) from CSS 3 Values and Units → Implement vw/vh/vmin/vmax (viewport sizes) from CSS 3 Values and Units
Comment 21•12 years ago
|
||
FWIW, here's a bug I filed on that Webkit behaviour.
https://bugs.webkit.org/show_bug.cgi?id=94805
I really hope that's a bug in both Webkit/MSHTML and not some legit spec interpretation, but if it is legit, worth linking here I suppose.
It's a bug.
And it's also why we've been saying this is actually not quite trivial to implement.
Comment 23•12 years ago
|
||
Sure sure. Not that rushed. Actually, if Webkit and MSHTML had just added the equiv of the javascript hack in that testcase (force some sort of recalculate everything every 200ms or so if the viewport is different from the size it was previously) I'd be quite fine w/ that. I doubt many people care about smooth resizes. I do care if stuff gets screwed up when a window is unmaximised or a device rotated ofc. Hell, I'd be fine personally if I could just find a semi-clean way to tell Webkit to force a recalc. Actually, IE is even worse on that front since the ugly method that works in Webkit does not work in IE :(
That being said, even the broken Webkit implemenation is better than no implementation at all :)
Hm. Interesting. I know the IE guys tend to skip -ms- 'cause no one ever bothers w/ IE prefixes, and the moz guys have been having the same problem as -o-pera w/ the goodies that Apple invented (amusingly Apple's transitions are completely broken in Safari forcing me to disable -webkit).
But... There's no equivalent for things like units. You guys would presumably prefer not to ship a broken half-assed implementation.
But there's not like a -moz-vw :)
Comment 24•12 years ago
|
||
Over to Seth for his second bug. Note: I was told seth@mozilla.com was already taken :(
Check with desktop IT for a different mail alias.
Assignee: nobody → seth
Assignee | ||
Comment 25•12 years ago
|
||
(In reply to Jet Villegas (:jet) from comment #24)
> Over to Seth for his second bug. Note: I was told seth@mozilla.com was
> already taken :(
>
> Check with desktop IT for a different mail alias.
Heh, it was taken by me. I'm proactive that way. =)
Assignee | ||
Comment 26•12 years ago
|
||
Here's an initial version of the patch for feedback. This version works fine with some of the test cases here and elsewhere (e.g. http://m8y.org/tmp/testcase272.xhtml above). Known issues are as follows:
(1) It doesn't work with the test case found in the attachment. This _seems_ to be due to an unrelated bug and not the code in this patch, but I still haven't tracked down the root cause yet. The text in the red table cells seems to force those cells to be much larger than in other browsers.
(2) I haven't added test cases yet.
Attachment #670565 -
Flags: feedback?(dbaron)
Assignee | ||
Comment 27•12 years ago
|
||
After talking with :dbaron I've resolved issue 1; the problem is related to table cells defaulting to 'vertical-align: baseline'. Some of the cells in the table have text, and some do not. The baseline of a cell without text is its top. Trying to align this with the baseline of cells that actually have text causes some ugly artifacts in the table rendering. This may work in Chrome / Safari because they don't implement 'vertical-align: baseline' correctly, though I have not verified this.
At any rate, the fix is to apply 'vertical-align: top' to all table cells. A modified test case is incoming.
Assignee | ||
Comment 28•12 years ago
|
||
Added missing 'vertical-align: top' CSS property to the table cells in the test case, making it render correctly in all browsers. (All browsers that support vw/vh, anyway.)
Attachment #538870 -
Attachment is obsolete: true
Assignee | ||
Comment 29•12 years ago
|
||
Updated patch with accompanying reftests. All known issues are taken care of now, and the test cases all look good. Requesting review.
Attachment #670565 -
Attachment is obsolete: true
Attachment #670565 -
Flags: feedback?(dbaron)
Attachment #670618 -
Flags: review?(dbaron)
Comment 30•12 years ago
|
||
May be worth noting that your update of the testcase has changed its MIME type from application/xhtml+xml to text/html, which has changed the rendering mode from standards to quirks.
Assignee | ||
Comment 31•12 years ago
|
||
@Patrick: Whoops, thanks for catching that. Corrected.
Attachment #670617 -
Attachment is obsolete: true
Not that it really matters much, but it's usually easier to use text/html and force standards mode with <!DOCTYPE HTML>.
Comment 33•12 years ago
|
||
You're right it doesn't matter much, but I personally use application/xhtml+xml to catch my mistakes *in* the doc on the fly, and also, occasionally, for respone doc in XHR, where it is darn convenient, and certainly a lot better than regex for parsing, and tidier than an iframe.
And, I have to say, I do wish XHTML5 instead of being some grudging afterthought to HTML5, had actually taken advantage of the tidiness to do stuff like, oh, <img src="format1"><img src="format2">Description and fallback <a href="foo">link</a></img></img> - possible w/ <img /> - not so much w/ <img> :-/
oh well, yeah, doesn't matter and offtopic.
Comment on attachment 670618 [details] [diff] [review]
Implement vw/vh/vmin/vmax.
> void
> nsPresContext::MediaFeatureValuesChanged(bool aCallerWillRebuildStyleData)
> {
> mPendingMediaFeatureValuesChanged = false;
> if (mShell &&
>- mShell->StyleSet()->MediumFeaturesChanged(this) &&
>- !aCallerWillRebuildStyleData) {
>+ !aCallerWillRebuildStyleData &&
>+ (mUsesViewportUnits || mShell->StyleSet()->MediumFeaturesChanged(this))) {
> RebuildAllStyleData(nsChangeHint(0));
> }
I think we probably want this to be better-optimized -- i.e., either (1) add a separate function or an additional parameter to MediaFeatureValuesChanged so we only do this rebuild for the callers that could change the size or (2) actually cache the last size we used for viewport units and only rebuild when it changes
>diff --git a/layout/style/nsCSSValue.h b/layout/style/nsCSSValue.h
> * between relative length units and pixel length units.
> *
> * A "relative" length unit is a multiple of some derived metric,
> * such as a font em-size, which itself was controlled by an input CSS
> * length. Relative length units should not be scaled by zooming, since
> * the underlying CSS length would already have been scaled.
> */
> bool IsRelativeLengthUnit() const
>- { return eCSSUnit_EM <= mUnit && mUnit <= eCSSUnit_RootEM; }
>+ { return eCSSUnit_ViewportWidth <= mUnit && mUnit <= eCSSUnit_RootEM; }
I think every caller of IsRelativeLengthUnit() actually wants false for these new units rather than true. (I think they all care about whether the unit is relative to a font size, since they're either asking because of text-zoom (which should zoom all text, but not zoom it twice) or whether the behavior of the unit is inheritance-like for 'font-size'.) The comment above the function could probably be improved.
>diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp
Did you test that you get the correct behavior when zooming? I think that's worth testing manually and adding a reftest for (see http://mxr.mozilla.org/mozilla-central/search?string=reftest-zoom ).
Otherwise I think this looks good, but I'd like to see the revisions for the first point.
Attachment #670618 -
Flags: review?(dbaron) → review-
Assignee | ||
Comment 35•12 years ago
|
||
Thanks for the feedback, David. I've made the changes you requested.
Specifically regarding the behavior while zooming, I did indeed check this and it works fine. (You CAN make the test case render in a ugly way if you zoom in really, really far, but that has more to do with the table layout code and is to be expected.) I've added a zoomed version of the reftest, which passes.
Attachment #670618 -
Attachment is obsolete: true
Attachment #671633 -
Flags: review?(dbaron)
Comment on attachment 671633 [details] [diff] [review]
Implement vw/vh/vmin/vmax.
r=dbaron
Attachment #671633 -
Flags: review?(dbaron) → review+
Keywords: checkin-needed
Comment 37•12 years ago
|
||
I don't see a Try link here, so I've pushed it myself. I'll land this if it's green.
https://tbpl.mozilla.org/?tree=Try&rev=e29c3fd60ea9
Updated•12 years ago
|
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Comment 38•12 years ago
|
||
Looks like this has build failures on Windows and mochitest failures on all platforms.
Keywords: checkin-needed
Assignee | ||
Comment 39•12 years ago
|
||
Thanks Ryan. Will have a look.
Assignee | ||
Comment 40•12 years ago
|
||
Ryan: was able to reproduce the mochitest failure; it boiled down to the order of two boolean checks in an if statement being reversed. I've modified the patch to fix that and it now passes the test. (I've commented things to make this issue more clear when people touch this code in the future.) I discussed this fix IRL with dbaron, and it's a tiny change, so I'm carrying over the r+.
dbaron and I have agreed that the relevant function could use some refactoring, but we'll address that in a separate bug.
I've got a try run going at https://tbpl.mozilla.org/?tree=Try&rev=415bcd4414f9. I'm not sure yet if the Windows failures were spurious or not; let's see if they reappear in this try run.
Attachment #671633 -
Attachment is obsolete: true
Attachment #672524 -
Flags: review+
Assignee | ||
Comment 41•12 years ago
|
||
Carrying r+ forward.
The problem appears to have been that Microsoft's compiler still, in 2012, does not fully support C99. This made compilation fail because of |fmin| and |fmax|. I switched over to C++'s |std::min| and |std::max|, which are probably a better choice anyway.
This should resolve the last remaining issue with this patch. I've got a try job going at https://tbpl.mozilla.org/?tree=Try&rev=91c40c4e1f0e to verify that. If all goes well with the try run I'll request checkin.
Attachment #672524 -
Attachment is obsolete: true
Attachment #673068 -
Flags: review+
Comment 42•12 years ago
|
||
Try run for 91c40c4e1f0e is complete.
Detailed breakdown of the results available here:
https://tbpl.mozilla.org/?tree=Try&rev=91c40c4e1f0e
Results (out of 248 total builds):
success: 234
warnings: 14
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mfowler@mozilla.com-91c40c4e1f0e
Assignee | ||
Comment 43•12 years ago
|
||
Excellent, try run looks good. Let's put this one to bed.
Keywords: checkin-needed
Updated•12 years ago
|
Severity: enhancement → normal
Comment 44•12 years ago
|
||
Flags: in-testsuite+
Keywords: checkin-needed
Comment 45•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
Comment 46•12 years ago
|
||
I've updated:
the reference article about <length> units: https://developer.mozilla.org/en-US/docs/CSS/length#Viewport-percentage_lengths
And the usual: https://developer.mozilla.org/en-US/docs/Firefox_19_for_developers
We are the first engine to implement this completely (relative to the CR)!
Keywords: dev-doc-needed → dev-doc-complete
Depends on: 811391
Depends on: 811403
Comment 47•12 years ago
|
||
May be worth noting that the test case still fails when the viewport is made sufficiently small (based on testing in Nightly). I'm not sure if this is by design or not.
Nightly seems to enforce a minimum layout width for the table at small window sizes. That causes the table to no longer be placed correctly relative to the absolute-positioned elements.
Comment 48•12 years ago
|
||
Patrick, are you sure the viewport (as opposed to to the browser window) is being made small?
A number of Firefox extensions break the UI such that it won't shrink below some minimum width, so that you can make the window smaller than the UI and smaller than the web viewport.
Assignee | ||
Comment 49•12 years ago
|
||
(In reply to Patrick Dark from comment #47)
> Nightly seems to enforce a minimum layout width for the table at small
> window sizes. That causes the table to no longer be placed correctly
> relative to the absolute-positioned elements.
I see what you mean, certainly. The test case could probably be better constructed so as not to fail at small window sizes. Viewport units do work correctly though. The problem is actually the text in the test. The presence of the text puts a lower bound on the size of the table cells, which causes the anomaly you're seeing. If you go through the DOM on the test page and delete every <p> node, you'll see that things behave as expected even with very small window sizes.
Comment 50•12 years ago
|
||
My bad; false alarm. Seth is right: the issue is a flawed test case that doesn't account for the minimum content width of table cells. I wrote a revised version, and Nightly passed that version without issue.
(I also noticed that three of the four |right| property values are wrong, but they're ignored, so those errors don't affect the result.)
You need to log in
before you can comment on or make changes to this bug.
Description
•