Closed Bug 809788 Opened 12 years ago Closed 12 years ago

window.devicePixelRatio should be unaffected by page zoom

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX
Tracking Flags:
Tracking Status
firefox18 - affected
firefox19 - affected

People

(Reporter: jfkthame, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 1 obsolete file)

window.devicePixelRatio should be unaffected by page zoom
3.45 KB, patch
Details | Diff | Splinter Review
test that window.devicePixelRatio is unaffected by zoom
2.30 KB, patch
Details | Diff | Splinter Review
It seems that webkit and opera's implementations of window.devicePixelRatio return a value that depends only on the display in use, not on the page zoom. See discussion in bug 809331. We should probably make ours match this.
This changes our implementation so that it is unaffected by zoom. We should have tests for this, I guess... I've pushed this to tryserver to see if it breaks any existing tests, for a start. https://tbpl.mozilla.org/?tree=Try&rev=7e10a5792ad0
Attachment #679597 - Flags: review?(jst)
Note that window.devicePixelRatio landed (in bug 564815) for mozilla18, so we haven't yet shipped it to the release channel. So if we're going to change its semantics to match other browsers, now's the time to do so.
Comment on attachment 679597 [details] [diff] [review] window.devicePixelRatio should be unaffected by page zoom Hmm, so that patch breaks the test in dom/tests/mochitest/general/test_innerScreen.xul that expects DOMWindowUtils.screenPixelsPerCSSPixel to respect the current zoom. Looking at bug 564815, the assumption there was that devicePixelRatio is just an alias for screenPixelsPerCSSPixel. Assuming we don't want to change the existing meaning of screenPixelsPerCSSPixel (which *does* respect zoom, as one would expect from its name - after all, zooming causes CSS pixels to change size on the screen), I think what we really need to do here is decouple these two properties. This makes reasonable sense, IMO: screenPixelsPerCSSPixel is about the relationship between CSS pixels and the screen, so it is affected by zoom, whereas devicePixelRatio is about the physical properties of the display, so it remains constant.
Attachment #679597 - Attachment is obsolete: true
Attachment #679597 - Flags: review?(jst)
Here's a version that decouples window.devicePixelRatio from utils.screenPixelsPerCSSPixel, as suggested above, so that the former is zoom-independent and the latter is zoom-aware. Tryserver job at https://tbpl.mozilla.org/?tree=Try&rev=9418153d0c15.
Attachment #679649 - Flags: review?(jst)
Rather surprisingly, that came up entirely green on try. Turns out the reason for this is that the test patch in bug 564815 wasn't landed yet, so we don't actually have any test coverage for window.devicePixelRatio. If that test had been in place, we'd have gotten a mochitest orange here due to the (intended) change in behavior. I'll attach an updated version of bug 564815's test patch, to account for the changed behavior; then we can simply obsolete the unlanded patch there, and use this bug's version instead.
Updated version of the (unlanded) test patch from bug 564815, to test the revised behavior of window.devicePixelRatio.
Attachment #679671 - Flags: review?(jst)
Nominating for tracking: window.devicePixelRatio is a new feature in mozilla18, but I believe the implementation as it currently stands is incompatible with webkit's (and opera's) already-shipping implementations, and hence also with web content that is being authored on that basis. We should fix it before we ship it to the release channel, IMO.
status-firefox18: --- → affected
status-firefox19: --- → affected
tracking-firefox18: --- → ?
tracking-firefox19: --- → ?
Tryserver run with the updated testcase: https://tbpl.mozilla.org/?tree=Try&rev=8d7f96f7638c
Are there specific benefits to making this change? Any specific content that is broken by the current behavior? The current (page-zoom-dependent) behavior improves the user experience in some cases. For example, sites like www.apple.com use higher-resolution images when devicePixelRatio is higher, which in Firefox 18 makes the pages look better both for users of page-zoom and users of high-density displays. I agree that compatibility would be good here, but have we asked WebKit developers whether they would consider changing their behavior instead?
The NYTimes map discussed in bug 809331 is an example that depends on the webkit semantics: it has code that specifically checks for devicePixelRatio==2 as the condition for doing high-res canvas drawing. So with our current implementation, it behaves one way for zoom=100%, and a different way for all other zoom factors, whether larger or smaller, which is a weird inconsistency. Depending exactly how a site tests the value, it's possible that our current behavior would cause them to switch to low-res images as soon as the user zooms out, even by a single step, as devicePixelRatio is then slightly less than 2. It's true that the current implementation means a site such as Apple's will switch to larger images for a non-Retina user if they zoom in sufficiently (probably at least 150% or more), but do we think users frequently browse in such a zoomed-in state? Note that we do *not* switch to using the high-res images on the fly as the user zooms, because that does not trigger a reload of the page, so we continue to render whatever resource was originally loaded. So this doesn't help a user who just wants to temporarily zoom in to look more closely at an image, unless they know to explicitly reload the page after zooming.
Thanks, that makes sense. I agree that browsing with very high page zoom is rare, so it doesn't make sense to optimize for that if there are compatibility costs.
(In reply to Jonathan Kew (:jfkthame) from comment #10) > The NYTimes map discussed in bug 809331 is an example that depends on the > webkit semantics: it has code that specifically checks for > devicePixelRatio==2 as the condition for doing high-res canvas drawing. So > with our current implementation, it behaves one way for zoom=100%, and a > different way for all other zoom factors, whether larger or smaller, which > is a weird inconsistency. I argue that the NYTimes site is broken. If a device comes out with devicePixelRatio > 2, wouldn't they want to serve the high-res map? This is actually a pretty severe problem for the future that we should try to get sites to avoid. > Depending exactly how a site tests the value, it's possible that our current > behavior would cause them to switch to low-res images as soon as the user > zooms out, even by a single step, as devicePixelRatio is then slightly less > than 2. Similar. Many actual mobile devices have a devicePixelRatio of around 1.5. Arguably NYTimes should serve the high-res map to them. > Note that we do *not* switch to using the high-res > images on the fly as the user zooms, because that does not trigger a reload > of the page, so we continue to render whatever resource was originally > loaded. So this doesn't help a user who just wants to temporarily zoom in to > look more closely at an image, unless they know to explicitly reload the > page after zooming. Zoom levels persist so if they visit the site, zoom in, then visit the site again later, they'll benefit. I think we're doing the right thing here already.
Also, I think we should clarify the compatibility issues here. As far as I can tell, the consequences of behaving differently are that we might not get the ideal-resolution content, but the page will otherwise work OK. The page layout will not be broken. (The breakage in bug 809331 is due to a different issue.)
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12) > (In reply to Jonathan Kew (:jfkthame) from comment #10) > > The NYTimes map discussed in bug 809331 is an example that depends on the > > webkit semantics: it has code that specifically checks for > > devicePixelRatio==2 as the condition for doing high-res canvas drawing. So > > with our current implementation, it behaves one way for zoom=100%, and a > > different way for all other zoom factors, whether larger or smaller, which > > is a weird inconsistency. > > I argue that the NYTimes site is broken. If a device comes out with > devicePixelRatio > 2, wouldn't they want to serve the high-res map? This is > actually a pretty severe problem for the future that we should try to get > sites to avoid. I'd agree NYT's page is broken. It would make more sense for them to check for devicePixelRatio >= 2, at least. It just happens that their code serves rather well to spotlight the design incompatibility between our devicePixelRatio and webkit's/opera's version. > > > Depending exactly how a site tests the value, it's possible that our current > > behavior would cause them to switch to low-res images as soon as the user > > zooms out, even by a single step, as devicePixelRatio is then slightly less > > than 2. > > Similar. Many actual mobile devices have a devicePixelRatio of around 1.5. > Arguably NYTimes should serve the high-res map to them. True - though whether to do higher-res canvas drawing, and what actual threshold to pick (1.5? 1.75? 2?) is a decision authors will need to take; serving the double-res map to such device could represent an undesirable burden on mobile bandwidth. Apparently the NYT authors either didn't consider such devices - most likely, IMO; the map page doesn't look like it was created with small-screen mobile users in mind - or they decided against serving them the high-res map. (Hmm, tried going there with an iPod Touch, and it looks like they serve a different version of the page altogether.) Still, they intended the high-res map for all retina-MBPro users; they didn't expect that a retina user who browses at 90% zoom would miss out on it, because they didn't expect devicePixelRatio to vary. > > Note that we do *not* switch to using the high-res > > images on the fly as the user zooms, because that does not trigger a reload > > of the page, so we continue to render whatever resource was originally > > loaded. So this doesn't help a user who just wants to temporarily zoom in to > > look more closely at an image, unless they know to explicitly reload the > > page after zooming. > > Zoom levels persist so if they visit the site, zoom in, then visit the site > again later, they'll benefit. That sounds like a very marginal use case. Moreover, I think this actually implies there is something wrong with how we're doing it. It means that if I go to site X and zoom in, I see one version of the page; if I then leave that site and return to it, I see something different. (Or maybe not, depending whether we actually reload the page or use it from cache?) Then I zoom back to the default view, and I don't see the original site - I see the high-res version downscaled, and it looks all "muddy". IMO, either we should be reloading the page when zoom changes (which I don't think we'd really want to do, as people expect zoom to be lightweight and instant), or zoom should not be affecting what's loaded in the first place - so it should not affect media queries etc.
(In reply to Jonathan Kew (:jfkthame) from comment #14) > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #12) > > Zoom levels persist so if they visit the site, zoom in, then visit the site > > again later, they'll benefit. > > That sounds like a very marginal use case. Moreover, I think this actually > implies there is something wrong with how we're doing it. It means that if I > go to site X and zoom in, I see one version of the page; if I then leave > that site and return to it, I see something different. (Or maybe not, > depending whether we actually reload the page or use it from cache?) Then I > zoom back to the default view, and I don't see the original site - I see the > high-res version downscaled, and it looks all "muddy". This is all because they're using JS to sample the devicePixelRatio just once. If they used something that automatically handled dynamic changes, e.g. media queries, none of these problems would happen. Or, they could handle the resize event and update the decisions they made based on devicePixelRatio. A similar situation would be if they used JS to read the window size on load, and made some layout decisions based on that. Then when the user resizes the window, we'd see the same sorts of problems. > IMO, either we should be reloading the page when zoom changes (which I don't > think we'd really want to do, as people expect zoom to be lightweight and > instant), Yes, we definitely aren't going to do that. > or zoom should not be affecting what's loaded in the first place - > so it should not affect media queries etc. You mean across the board, such as for media queries on viewport size? I don't think that would make sense.
BTW, I think this discussion probably belongs on www-style.
Started a thread there titled "Behavior of window.devicePixelRatio under zoom".
Clearing the tracking flags for now. Please renominate this bug after we have a consensus on this issue which is being discussed as per comment 17 .
tracking-firefox18: ? → -
tracking-firefox19: ? → -
Comment on attachment 679649 [details] [diff] [review] window.devicePixelRatio should be unaffected by page zoom As for the code changes here goes, this looks all good to me, but it seems it's not clear what the right fix is here. Clearing review flags until that's been settled.
Attachment #679649 - Flags: review?(jst)
Attachment #679671 - Flags: review?(jst)
I think the www-style thread is veering towards a consensus that we shouldn't change our behavior here.
Yes - it sounds like we should stick with our (and IE9's) existing behavior, and expect/hope that webkit and opera will bring their implementations into line.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
I don't want to troll this issue, but implementing a feature the wrong way *after* everybody else, use IE as an excuse, and wait for the others to get in line? Really? Zoom level is different than devicPixelRatio and they shouldn't mix. Give the user a way to get the zoom level separately and the pixel ratio separately and stop the fragmentation of implementation before it starts.
Website developers just begun to take care of pixel-ratio aspects, thanks to the retina stuffs. Do you really think they are going to consider the zoom-level if this notion is split from screen density ? This is nonsense. Matching the pixel-ratio to the zoom-level is just the good way to do, because zooming is nothing but a screen density modification. Not only Firefox should keep this behaviour, but it should propose it as a standard to bring its benefits to the small proportion of users who need it, namely people with disabilities, too often neglected by web developers. You should try maps.google.com against Chrome and Firefox both zoomed at 200% and see the difference.
Yes they will care, because pixel ratio is a system property, it is forced on the user, and zoom level is an accessibility option that can be triggered by the user. By not giving the developer any way to distinguish between the two you are actually hurting the user because the device aspect ratio is used mainly to give the same experience across platform where detecting a zoom level can help the developer give a better experience to people who use or need this accessibility feature (for example - a warning strip that a certain app will not preform as well when zoomed like google docs, or an online editor that if you edit a site when zoomed it will not look the same to your end user, etc.) I'm maintaining this utility - http://github.com/tombigel/detect-zoom - that more and more people use just for this purpose and right now, the only browser that I can't give a good indication on is Firefox.
Blocks: 847077
Blocks: 843767
I really cannot believe that this issue is set to 'WONTFIX'! As RWD becomes more and more important and as so also the usage of media queries and other JS based solutions it is very annoying to me as a webdesigner that IMHO you are doing it all wrong - sorry to say that. But apart from all specs and standards it seems logically clear to me that all stuff related to 'device...' should not be changed by any user performed action like zooming the page as the device itself stays the same! BTW: You are doing it in the same wrong way with 'device-width/height' which with no doubt should also not be affected by page zoom! So please corrected these obviously wrong behaviors ASAP - many thanks!
Gunther, devicePixelRatio is the ratio of CSS pixels to device pixels. When zooming, device pixels stay the same but CSS pixels change. device-width/height is measured in CSS pixels in CSS. So for example on a HiDPI screen device-width will be the "scaled" resolution the OS is exposing, not the actual resolution of the panel. Again, zoom changes the size of a CSS pixel. So the behavior is in fact correct; you just have a very confused mental model of how this stuff works.
This is a real problem, especially for HTML5 canvas. When the device pixel ratio !=1, canvas applications typically adjust the size and scale of the canvas in order to produce crisp drawings. This technique works for just about every device in existence, except for Firefox when the zoom != 100%
(In reply to Eric Rowell from comment #27) > This is a real problem, especially for HTML5 canvas. When the device pixel > ratio !=1, canvas applications typically adjust the size and scale of the > canvas in order to produce crisp drawings. This technique works for just > about every device in existence, except for Firefox when the zoom != 100% Isn't that good? If you use that canvas technique properly, then you'll produce crisp drawings in Firefox when the zoom != 100% ... right?
This is unacceptable. Developers right now have no way of distinguish whether the user is zoomed in or using a Retina display. The name "devicePixelRatio" implies it relies on the specs of the device, and the zoom.
Is this issue somehow resolved on Firefox?
Blocks: 1022006
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: