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)

defect
Not set
normal

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)

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
Is "width: 100%" the same as "width: 1vw"?
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.)
Also, 100vw is the full width of the viewport, not 1vw.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
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.
Attached file Test Case for |vw| and |vh| Units (obsolete) —
(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
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?
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.
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).
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.
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
Update on implementation: these units have been implemented in Chrome (not sure since which release) and Safari 6 Beta.
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.
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.
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.
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)
> 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
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.
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 :)
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
(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. =)
Attached patch Implement vw/vh/vmin/vmax (obsolete) — Splinter Review
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)
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.
Attached file Test Case for |vw| and |vh| Units (obsolete) —
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
Attached patch Implement vw/vh/vmin/vmax. (obsolete) — Splinter Review
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)
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.
@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>.
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-
Attached patch Implement vw/vh/vmin/vmax. (obsolete) — Splinter Review
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+
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
Status: NEW → ASSIGNED
Version: unspecified → Trunk
Looks like this has build failures on Windows and mochitest failures on all platforms.
Keywords: checkin-needed
Thanks Ryan. Will have a look.
Attached patch Implement vw/vh/vmin/vmax. (obsolete) — Splinter Review
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+
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+
Blocks: 802849
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
Excellent, try run looks good. Let's put this one to bed.
Keywords: checkin-needed
Severity: enhancement → normal
https://hg.mozilla.org/mozilla-central/rev/54af2c396f5f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
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)!
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.
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.
(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.
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.)
Depends on: 1475491
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: