Rex has a work-in-progress patch that enables WVGA and QHD support for Gaia. To prevent further maintenance burden, I would like to have the patch landed at the earliest when the tree is ready to receive non-v1 changes. After that, we should establish a rule in Gaia to maintain the support of these screens. Vivien, what's the earliest convenience for such changeset to land?

The changes are relatively wide (mainly on css that changes unit from px to rem and resize images), we may need some effort on reviewing and revising.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #0) > Rex has a work-in-progress patch that enables WVGA and QHD support for Gaia. > > To prevent further maintenance burden, I would like to have the patch landed > at the earliest when the tree is ready to receive non-v1 changes. > > After that, we should establish a rule in Gaia to maintain the support of > these screens. > > Vivien, what's the earliest convenience for such changeset to land? The tree won't be ready to receive non v1 change until we create a v1 branch. Sadly I don't know when this will happen (I really would like :/). So this one should likely land in the same time frame than shira.

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #2) > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #0) > > Vivien, what's the earliest convenience for such changeset to land? > > The tree won't be ready to receive non v1 change until we create a v1 > branch. Sadly I don't know when this will happen (I really would like :/). > > So this one should likely land in the same time frame than shira. What can we do to make it happen sooner?

https://github.com/rexboy7/gaia/commits/resolution_landing Just arranged the whole branch based on master of 1/24 here. Basically there's one commit for each app. After discussing with Tim, I would send a pull request and post each commit a patch file here for reviewing. What this patch mainly do is: - Put a shared css file called resolution.css, which is included in every apps, for detecting phone resolution and set font-size on body correspondingly. - Change unit from px/em to rem. - set background-size for picutres which need to be scaled on larger resolution. with miscellaneous changes. The work started about one month ago with several merges, I tried my best to catch new features need to patch and you may notice me if I missed. I'll start to send review request later.

(In reply to KM Lee [:rexboy] from comment #4) > https://github.com/rexboy7/gaia/commits/resolution_landing > Just arranged the whole branch based on master of 1/24 here. Basically > there's one commit for each app. After discussing with Tim, I would send a > pull request and post each commit a patch file here for reviewing. > What this patch mainly do is: > - Put a shared css file called resolution.css, which is included in every > apps, for detecting phone resolution and set font-size on body > correspondingly. > - Change unit from px/em to rem. > - set background-size for picutres which need to be scaled on larger > resolution. > > with miscellaneous changes. > The work started about one month ago with several merges, I tried my best to > catch new features need to patch and you may notice me if I missed. I'll > start to send review request later. We agreed with Andreas Gal that we will land the code developed for the Geeksphone Peak device that is already handling multiple resolution after freezing 1.0.0. Can we coordinate on this before landing anything?

I am not aware of any development on the Geeksphone Peak device -- until yesterday actually. Nonetheless, I am happy as long as this bug is fixed. Let's coordinate -- please suggest how we should do it.

So, Daniel, couple of questions on works from your devs: 1) Which branch would you intend to put the multi-resolution support? Specifically, which version of Gaia is the Geeksphone Peak devices will be using? Is it v1-train, master, or a specific v1.x version only? 2) Is the WIP code publicly available somewhere? 3) What's the target resolution the code is supported currently? Taipei would like to target at least QHD and WVGA. 4) What else does the code do? I am told that some magic has been done on build script We should be targeting landing a version where all the feature and the requirements is included, landed, and maintained. Thanks.

Seems best (this is standard practice for Gecko changes) always to go into mozilla-central (via mozilla-inbound) for Gecko and master for Gaia first, no? Or are those out of date and in need of merge-back from mozilla-b2g18 and v1-train? /be

(In reply to Brendan Eich [:brendan] from comment #8) > Seems best (this is standard practice for Gecko changes) always to go into > mozilla-central (via mozilla-inbound) for Gecko and master for Gaia first, > no? > > Or are those out of date and in need of merge-back from mozilla-b2g18 and > v1-train? > > /be Yes, everything will be checked into master first. I simply want to know how many branches Tef is going to land, for this feature.

Hi Tim Ismael has been working in this branch: https://github.com/basiclines/gaia/tree/twist-nightly As you can see there are some changes in gaia, and apps (not that many). Any way we will need to coordinate how we are going to move this changes to master. Thanks!

Yes, sorry, I forgot to follow-up on this one. Ismael has currently sync that branch with v1.0.0, but he could easily put it in sync with master or v1-train, whatever is required. Ismael has mostly changes in CSS and some minor things required to have a good UX (e.g. changing the number of icon rows in the grid). The branch includes graphical resources already updated to other resolutions, it is designed in a way that you can decide the resolution when building Gaia, e.g. SCREEN_TYPE=qHD make install-gaia or SCREEN_TYPE=wvga make install-gaia.

Hi all guys just to to clarify the work done on twist-nightly branch: * Added assets with double resolution (name@2x.png) for all the system and core apps * Background size declarations to handle this new assets * Makefile changes to decide when to send @2x assets or when to do not. * Changes in a couple of applications that make some JS calcs based on 320x480 device * Window manager JS changes for scaling the content of wrapped apps All this changes has been done in Gaia because of https://bugzilla.mozilla.org/show_bug.cgi?id=828531 Gecko should be responsible in returning more CSS pixel per Device Pixels, this means that if we can fix the above bug the only necessary changes in Gaia will be: * Add assets with double resolution (name@2x.png) for all the system and core apps * Background size declarations to handle this new assets * (optional)Makefile changes to decide when to send @2x assets or when to do not. With this approach is no longer needed to use "rem", we could keep working on "px" as Gecko will give us the proper amount of pixels. Also any web content loaded from Browser app or any wrapped apps will be scaled meanwhile with the Gaia "scale hack" will be much difficult because of this https://bugzilla.mozilla.org/show_bug.cgi?id=827802 I hope this info helps everyone

Ok. I think we need to start looking at those changes and land them on master now. Ismael and Rexboy can you sync together in order to take the best of the 2 worlds and to have only one branch? I gave a quick look at both branch and here are some questions: - There are some background-size: 3rem; on bitmap images. Maybe having more media queries for this? Or does SVG could be a solution for that? - There are some css properties where the usage of rem can be discussed too (height, width, left, top, transform). Such values means the UI will fully change based on font-size. Basically you assume: one resolution == one font size, and I'm not 100% convinced this will always be true if we want to make some apps more comfortable to use for people with bad eyes. For example if an option in Settings let people configure ask for a bigger font-size (or is there a way to do that with a transparent preference?). Asking dbaron. - Ismael, is the reason to add a new build option is because you can't emulate device-width on Firefox responsive design tool so you prefer to include only one css file instead of having a resolution.css file like in RexBoy branch? If yes (and I tend to think even if no) let's see if the devtools team can help or have anything to propose? Adding Mihai to the discussion but I guess roc can answer if there is any way to do it at all? Ismael I'm not 100% sure we want to use the layout.css.devPixelsPerPx preference of bug 828531. Let's ask roc about it. I may have more questions at some point but let's work on the first set of questions already :)

Ok, so let me explain you guys some points: - Gaia responsive UI work: * background-size rules allow us to define bigger bitmap assets (double size of our current ones), and use them for a variety of resolutions from "1 to 2" scales factor, this mean that the same image could be used for a Peak device (540x960, scale factor 1.6785) and for a Samsung Galaxy (480x600, scale factor 1.35), and we won't need to use different images for different devices. Using SVG was discard because of low performance issues, as much SVG as you have the lower response of the device. * About rem usage, the problem was on defining the html font-size to 10px, if we define 62.5% instead of 10px, we will have the 10px=1rem equality and we will allow to the user to change any preference to increase the size of the UI (via browser default font-size). I.E Peak font-size: 104% instead of 16.8px * About resolution.css, that was my first approach (instead of having different screens css files qhd.css, fwvga.css, etc..). This approach has some issues when you are using iframes inside your application. I.E communications and the contacts tab, the loaded content will have a different viewport that will not match the proper mediaquery. ( 320x460 (20px statusbar) comms viewport , contacts in comms 320x420 (40px of tabs)). So if any application use internal iframes will not match correctly. That is the reason to do all this work on build process. * I'm also doing some work with images at build process (webap-zip.js), only sending to the phone the @2x version (renaming it with the original filename), this avoid having to maintain hundred of CSS files with mediaqueries for image replacing. * Gaia responsive UI work is not reflected on mediaquieries (min-resolution) or window.devicePixelRatio, both of them return static values right now (96 and 1), also our window.innerWidth & window.innerHeight will return device pixels instead of CSS pixels (scaled ones). That means we will have compatibilities problems with some websites/external apps. * Any change in Gecko that assumes to read the dpi (remember right now returns 96) of the device will not be reflected and we will have upcoming bugs (right now we have some problems in Peak device with touch areas because of https://bugzilla.mozilla.org/show_bug.cgi?id=823619) - Gecko responsive UI work, this should be the way to follow, why? * To avoid the last 2 points from the previous explanation (Gaia responsive UI work) * Requires lees change sin Gaia (just bigger assets and some background-size) * W3C specification (i can't find the link right now) suggest that is the UA who has to manage this scale work if the device dpi is much larger than normal (normal is 160 in our phones but Peak almost have 250). * Some ideas that are being discussed right now in the W3C about device adaptation http://dev.w3.org/csswg/css-device-adapt/ Remember that Gecko right now is ready to do all this works, it has some bugs, but is more less ready: https://bugzilla.mozilla.org/show_bug.cgi?id=828531 Thx!

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #13) > - Ismael, is the reason to add a new build option is because you can't > emulate device-width on Firefox responsive design tool so you prefer to > include only one css file instead of having a resolution.css file like in > RexBoy branch? If yes (and I tend to think even if no) let's see if the > devtools team can help or have anything to propose? Adding Mihai to the > discussion but I guess roc can answer if there is any way to do it at all? Paul Rouget did a lot of the work on the dev tools UI (most of it?). I do not have much experience with complex UIs, but if you have any specific questions please let me know - I can try to help.

Please read the discussion in bug 796490 before replying :-). I think it's very important that we leverage Gecko here and set layout.css.devPixelsPerPx to something other than 1.0 for high-DPI devices, and fix any Gecko bugs you encounter with that. You should be able to use px units for your UI and get results that look visually reasonable no matter what the device DPI is. This is the way the Web works and FFOS should follow. Device-pixel screen size and device-pixel density are separate issues. Here it seems we're mostly talking about density although in the bug summary, "QHD" and "WVGA" are about device-pixel screen sizes.

Let me do some explain on resolution.css: As far as I know when using media query, width and height do always depends on iframe width and height thus causes problem on apps using iframe. But we can use device-width and device-height instead, which depends on real device width/height no matter the iframe size to overcome the problem. Another issue is that orientation properties in media query also depends on iframe width/height, so portrait/landscape mode really determines whether it's an "upright" iframe, which is not what we wanted. So at last I determine whether it's landscape or portrait by watching whether device-aspect-ratio is greater/lesser than 1. I found the problem above when tuning statusbar (an Iframe with 320x20px). Isamel is this the problem you encountered? I didn't find a way to solve wrapped apps problem Ismael mentioned. That's a great work.

Hi Ismael, It looks like the part requires more clarification is about devicePixelRatio. I agree with what roc said in comment 16, however that means more work for qHD support. Is it possible, with Rexboy's help, we could land WVGA support first in master, in this week? Taipei is going away for one week holiday next week. Tell me what you need and I'll see what I could do.

@Mihai Thx you, right now i think the developers tools cover all our needs in this matter. @Vivien, having changes in build process is not about i can't emulate device size in emulator @Robert i'm totally agree with you, and yes we are basically talking about dpi. @KM Lee Yes that the problem that i found :D, anyway using this resolution.css (or qhd.css, etc...) is an attempt to emulate the scale process in Gaia side, as @Robert pointed this should be done at Gecko level. @Tim landing wvga is the same as landing qhd, as the changes i can imagine are traversals for both resolutions. So we should know how many Gecko effort do we need to fix layout.css.devPixelsPerPx, in that way the things that we can already start landing (both compatible with Gecko scale and Gaia scale approach) are background-size declarations and @2x assets. Also i will like to know @Vivien opinion about having some build process to determine when to send @2x assets to the phone or when to do not. (this is already implemented on twist-nightly branch) or it must be the applications CSS files which with mediaqueries should determine it (remember we need support for min-resolution to follow this approach)

(In reply to Ismael from comment #19) > @Tim landing wvga is the same as landing qhd, as the changes i can imagine > are > traversals for both resolutions. > So we should know how many Gecko effort do we need to fix > layout.css.devPixelsPerPx, > in that way the things that we can already start landing (both compatible > with Gecko scale and Gaia scale approach) are background-size declarations > and @2x assets. OK. That's good to know. I am a bit confused and concerned on what you expect the Gecko should do. Reading your comment in the code here: https://github.com/basiclines/gaia/blob/twist-nightly/shared/screens/fwvga.css > B2G always returns with 'min-resolution' 96dpi instead of real dpi (equal to Macbook Pro 13' dpi's) A) This is a bug that should be fixed. However, this should not affect our ability to target screens with min-device-width/height. > B2G always returns with 'min--moz-device-pixel-ratio' 1 instead of real pixel ratio. B) This is also a bug that should be fixed in Gecko, However, this should not affect our ability to support WVGA since it is still = 1. (It's 2 in qHD though) > We are not using any media query because of applications internal iframes size will not match the device query C) min-height will not match the size of the screen, and it's _by design_. What will match is always min-device-width/height. Also, read bug 796490. roc, correct me if I am wrong, |layout.css.devPixelsPerPx| will scale up/down the entire web content away from the device pixel grid, which will make the image blurry... it's not something we are looking for, and we shouldn't expect Gecko to do this for web content. That's what I meant we can land WVGA support without Gecko fixing (A) and (B). |rem| trick will work here and we can depend on that even after Gecko fixes those bugs. Additionally, having Makefile to delete the @2x files is a great feature that we should land it. Let's start land WVGA support by submitting review requests for CSS/JS changes app by app. It's that OK for you, Ismael? Fake QHD support can also be included by targeting until we find out how to fix (B) in Gecko. roc, I believe min-resolution media query is hooked to |layout.css.dpi|, is that correct? What CSS unit will the perf affects? With the information we could solve (A). And, how do we correctly set moz-device-pixel-ratio to 2? |layout.css.devPixelsPerPx| is not the answer here, obviously.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #20) > Also, read bug 796490. roc, correct me if I am wrong, > |layout.css.devPixelsPerPx| will scale up/down the entire web content away > from the device pixel grid, which will make the image blurry... it's not > something we are looking for, and we shouldn't expect Gecko to do this for > web content. Most Web content scales fine. We snap borders etc to pixel edges to avoid blurriness in most cases. For images, you can avoid scaling the image by providing a higher-resolution image. For example if devPixelsPerPx is 1.5 and you provide a 150x150 image for an element with background-size:100px 100px, Gecko won't need to scale it and it'll look great. The effect of setting layout.css.devPixelsPerPx is very much like full-zoom in desktop Firefox. So it's easy to experiment to see how things will look. > roc, I believe min-resolution media query is hooked to |layout.css.dpi|, is > that correct? What CSS unit will the perf affects? With the information we > could solve (A). And, how do we correctly set moz-device-pixel-ratio to 2? > |layout.css.devPixelsPerPx| is not the answer here, obviously. Actually it is.

BTW min-resolution is a bit confusing in CSS because "min-resolution:Ndpi" refers to CSS inches, not physical inches, so it doesn't correspond to the "actual DPI" of the screen. device-pixel-ratio is less confusing.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #21) > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #20) > > Also, read bug 796490. roc, correct me if I am wrong, > > |layout.css.devPixelsPerPx| will scale up/down the entire web content away > > from the device pixel grid, which will make the image blurry... it's not > > something we are looking for, and we shouldn't expect Gecko to do this for > > web content. > > Most Web content scales fine. We snap borders etc to pixel edges to avoid > blurriness in most cases. For images, you can avoid scaling the image by > providing a higher-resolution image. For example if devPixelsPerPx is 1.5 > and you provide a 150x150 image for an element with background-size:100px > 100px, Gecko won't need to scale it and it'll look great. > > The effect of setting layout.css.devPixelsPerPx is very much like full-zoom > in desktop Firefox. So it's easy to experiment to see how things will look. > > > roc, I believe min-resolution media query is hooked to |layout.css.dpi|, is > > that correct? What CSS unit will the perf affects? With the information we > > could solve (A). And, how do we correctly set moz-device-pixel-ratio to 2? > > |layout.css.devPixelsPerPx| is not the answer here, obviously. > > Actually it is. Thanks for the clarification. That means we will be using |layout.css.devPixelsPerPx = 2| for HiDPI devices, and fix OOM/OTMA issues with it, which Ismael have already filed some bugs. Ismael, I am still not hear anything from you since comment 20... are you occupied on other works? Do you need Rex to take over and set review/land your patches instead?

oops, s/OOM/OOP/, out-of-process (rendering) issues.

(In reply to Vivien Nicolas (:vingtetun) (:21) from comment #13) > - There are some css properties where the usage of rem can be discussed too > (height, width, left, top, transform). Such values means the UI will fully > change based on font-size. Basically you assume: one resolution == one font > size, and I'm not 100% convinced this will always be true if we want to make > some apps more comfortable to use for people with bad eyes. For example if > an option in Settings let people configure ask for a bigger font-size (or is > there a way to do that with a transparent preference?). Asking dbaron. I'm not sure what the question is, though the use of 'rem' for resolution-independence has always seemed pretty fishy to me. (If you want to ask me questions, better to use needinfo?.)

Given the fact we should be tweaking Gecko for support HiDPI devices, I am split this into two bugs.

> (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #13) > > Basically you assume: one resolution == one font size IMO this is incorrect. For a certain size, we will be targeting a range of screen dimensions with media queries. The goal is not to make the physical size of the text look exactly the same on every device. > > For example if > > an option in Settings let people configure ask for a bigger font-size (or is > > there a way to do that with a transparent preference?) I don't think accessibility is the topic of the bug here, though I agree the fix here shouldn't limit our options. (In reply to David Baron [:dbaron] from comment #25) > I'm not sure what the question is, though the use of 'rem' for > resolution-independence has always seemed pretty fishy to me. It looks fishy to me too. However I cannot come up with a better alternative, without wrapping a large part of style into media query block for each resolution. That's just a trick people come up so that we could target a resolution in 1 lines like @media (...) { html { font-size: <new rem base number here>px } } Until there is a better alternative ...

@Tim Sorry for not replaying again, i haven't so much time, but i've been reading you guys. > A) This is a bug that should be fixed. However, this should not affect our ability to target screens with min-device-width/height. > B) This is also a bug that should be fixed in Gecko, However, this should not affect our ability to support WVGA since it is still = 1. (It's 2 in qHD though) This affects external developers ability to swap images for different resolutions, but we can go without it by now if we use the makefile to send that kind of images to the phone (already implemented on twist-nightly). > C) min-height will not match the size of the screen, and it's _by design_. What will match is always min-device-width/height. I'm agree on using only one resolution.css (with mediaqueries for device-height) instead of qhd.css or fwvga.css, seems pretty much nicer. @Robert > The effect of setting layout.css.devPixelsPerPx is very much like full-zoom in desktop Firefox. So it's easy to experiment to see how things will look. This also provides mediaqueries and devicePixelRatio to return the correct values. @Tim, @David > I'm not sure what the question is, though the use of 'rem' for resolution-independence has always seemed pretty fishy to me. (If you want to ask me questions, better to use needinfo?.) > I don't think accessibility is the topic of the bug here, though I agree the fix here shouldn't limit our options. I think @Vivien question was more/less clarified in the above comments, we can keep accessibility features and bulletproof scaled UI: > * About rem usage, the problem was on defining the html font-size to 10px, if we define 62.5% instead of 10px, we will have the 10px=1rem equality and we will allow to the user to change any preference to increase the size of the UI (via browser default font-size). I.E Peak font-size: 104% instead of 16.8px So, final conclusions, just to clarify: 1) We should land WVGA with Gaia patches (me and rexboy) 2) We will fix the gecko (when?) bugs for using layout.css.devPixelsPerPx in QHD resolutions and any other one

(In reply to Ismael from comment #28) > So, final conclusions, just to clarify: > 1) We should land WVGA with Gaia patches (me and rexboy) > 2) We will fix the gecko (when?) bugs for using layout.css.devPixelsPerPx in > QHD resolutions and any other one YES. Thank you :)

I think we are mixing two topics here: 1/ The fixes in Gecko (that should be done for any resolution that is not the standard one HVGA) 2/ The adaptations in Gaia to support multiple resoultions, including the hack to bypass Gecko limitations for the time being. These adapations currently work for multiple resolutions (including WVGA and qHD) This bug should be about landing 2/ so that Gaia can already work for those resolutions. qHD is essential to be landed asap as it is already used in Firefox OS developer devices. Ismael will work starting tomorrow on 2/ as he has been focused so far in finishing the version that will be used for FirefoxOS Peak device for mass production.

If we decide to use resolution.css just include it in index.html need it and things should work. I did a quick merge putting resolution.css on and remove wvga.css/qhd.css and seems there are no significant UI break.

(In reply to Daniel Coloma:dcoloma from comment #30) > 2/ The adaptations in Gaia to support multiple resoultions, including the > hack to bypass Gecko limitations for the time being. What hack is that, exactly? Also, when you say "resolution", do you mean screen size or do you mean pixel density? I think it would be best to not use the word "resolution" at all :-)

> What hack is that, exactly? @Daniel means the work done by rexboy and me on Gaia to emulate the scaled UI (with rem units), until we have a better approach from Gecko side

Why exactly can't we just set layout.css.devPixelsPerPx right now and proceed without the rem hack? I'm aware of bug 828531. We have a developer working on it and I expect it will get fixed very soon. In the meantime it shouldn't block your work since it only happens while transitions/animations are running.

(In reply to Ismael from comment #33) > > What hack is that, exactly? > @Daniel means the work done by rexboy and me on Gaia to emulate the scaled > UI (with rem units), until we have a better approach from Gecko side Ismael, you are not reading my comment 27. rem is not a "hack", is a CSS unit that we are going to leverage to get Gaia ships in multi-dimension with the smallest possible media query code. Unless you got a better alternative... |layout.css.devPixelsPerPx| and other Gecko work will not replace that, and it's legit use for us to support HiDPI device will be addressed in bug 838505. Taipei is slip into long holiday in like 3 hours ... we really don't have time for this kind of miscommunication. (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #34) > Why exactly can't we just set layout.css.devPixelsPerPx right now and > proceed without the rem hack? Repeat, we are NOT going to adjust |layout.css.devPixelsPerPx| to a non-integer to resize Gaia. We should resize Gaia in CSS, as all mobile web developer do.

"rem" is a hack because CSS provides a resolution-independent unit, which is "px", and that's the unit we should be using. > Repeat, we are NOT going to adjust |layout.css.devPixelsPerPx| to a non-integer > to resize Gaia. Why not? > We should resize Gaia in CSS, as all mobile web developer do. I don't know what you're referring to.

:timdream I've read your comment #27 and I'm agree in using 'rem' and a resolution.css with "%" instead of "px" for satisfying :vingtetun suggestions. Anyway I'm totally agree with :roc, Gecko should be the responsible for it. This matter is enough important to give it a chance to discuss properly. Maybe we should involve more people from platform to discuss it. Meanwhile i will start landing the fixes needed for both scales methods (from Gecko and Gaia, background-size css declarations, higher resolution assets, and some Makefile additions) By the way :roc still using 'rem' is not a bad idea, as it is directly relational with "px".

What's the question?

[:cjones] The question is: Should be Gecko responsible via layout.css.devPixelsPerPx for scaling Gaia or it should be Gaia the responsible for it via "rem" and root font size adjusts?

Generally I want to do whatever roc suggests :). However, we have to ensure that pages like games can "see" the real resolution.

For canvas to work optimally with layout.css.devPixelsPerPx != 1 we need to fix bug 780362. We can prioritize that if it's needed. However, existing canvas code should work OK with layout.css.devPixelsPerPx != 1, it'll just be a bit less crisp.

Comment on attachment 712479 [details] PR the proposed approach for dealing with 2x resources looks good to me, although it would need thorough review from makefile owners. Concerning discussions on which layer should be scaling +1 to do in Gecko as it is more a platform issue than a Gaia issue. However, I still believe well-designed apps should define measurements in rem and not in px, just to enable future accessibility or zooming requirements. Concerning the games issue pointed out by Chris I think that's a very good point and we should ensure that automatic translation between logical and physical pixels do not break other apps.

(In reply to Chris Jones [:cjones] [:warhammer] from comment #41) > Generally I want to do whatever roc suggests :). However, we have to ensure > that pages like games can "see" the real resolution. Shouldn't this be controlled by using proper meta viewport directives?, like target-densitydpi=device-dpi which has been available in Android for a long time. http://developer.android.com/guide/webapps/targeting.html There are related discussions in Webkit at https://lists.webkit.org/pipermail/webkit-dev/2012-May/020847.html and related bug at https://bugs.webkit.org/show_bug.cgi?id=88047

(In reply to Jose M. Cantera from comment #43) > the proposed approach for dealing with 2x resources looks good to me, > although it would need thorough review from makefile owners. Concerning > discussions on which layer should be scaling +1 to do in Gecko as it is more > a platform issue than a Gaia issue. However, I still believe well-designed > apps should define measurements in rem and not in px, just to enable future > accessibility or zooming requirements. You can use rem as well as setting layout.css.devPixelsPerPx, if you want, but that seems like overkill. > Concerning the games issue pointed out by Chris I think that's a very good > point and we should ensure that automatic translation between logical and > physical pixels do not break other apps. We have a lot of experience with varying devPixelsPerPx on desktop since it's exactly the same mechanism that we use for "full zoom" of Web pages. It generally works well.

Comment on attachment 712479 [details] PR Hello, the media query seems OK to me but a little doubt if it works on both landscape/portrait mode. You may see the PR on github. Thanks!

Comments addressed :D

Sorry for not looking at this sooner - I'm busy with some performance issues and I have an hard time to focus on the review. I feel like Tim can do it (and fwiw I feel like we should follow roc suggestions about the pref).

Comment on attachment 712479 [details] PR The general approach of this part is correct, but I will strongly against we land this as-is: 0) Various typo and grammar error in comments. Having ambiguous comment is worse than having no comment. 1) Instead of rewriting settings.py (why would you do that?), please pass the HIDPI parameter to the files, and patch the code respond to the parameter. 2) Why would you need to rewrite the Makefile? 3) The two checks in webapp-zip.js looks like duplicates from me. I thought the only thing you want to do in addToZip() is -- if (HIDPI && this_is_1x_file && the_2x_file_exists ) return; -- if (!HIDPI && this_is_2x_file && the_1x_file_exists ) return; right? 4) I suppose responsive.css will be included to all apps as shared style. That works. 5) I am not sure the 94% in |html { font-size: 94%!important; }| is intentional or not. 94% of what value you are referring too? Looks to me this will result the font-size being set to 15.033px, which is 94% of the ua default (16px). I would recommend setting the value to a literal pixel value. 6) In the media query, |orientation| should be |device-aspect-ratio|. |orientation| is by design responsive to the size of the app iframe, not the real orientation of the phone screen. 7) Please remove the qHD entry in responsive.css, as the setting right now is a incorrect workaround. for qHD device (540×960, device-pixel-ratio=2), Gaia should resize to 270x480 (css-)pixels, and have Gecko to map 2 device pixels to 1 css pixel -- but all that should be addressed in bug 838505, not here.

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #45) > (In reply to Jose M. Cantera from comment #43) > > the proposed approach for dealing with 2x resources looks good to me, > > although it would need thorough review from makefile owners. Concerning > > discussions on which layer should be scaling +1 to do in Gecko as it is more > > a platform issue than a Gaia issue. However, I still believe well-designed > > apps should define measurements in rem and not in px, just to enable future > > accessibility or zooming requirements. > > You can use rem as well as setting layout.css.devPixelsPerPx, if you want, > but that seems like overkill. That's exactly my intention, and no, that's an overkill. Web content (e.g. Gaia), especially web apps, should always be fluid to device dimensions, and web platform (e.g. Gecko) should be reliable enough to expose the right parameters to web contents. A qHD device by definition is 540×960 (physical-)pixels and device-pixel-ratio=2. What I strongly against was to have Gaia ignore all that differences (and remove the rem work), and set Gecko pref of device-pixel-ratio to an arbitrary number (say, 2.232) just because the font look too small. I know Gecko should works well with the pref, but that's not right. > We have a lot of experience with varying devPixelsPerPx on desktop since > it's exactly the same mechanism that we use for "full zoom" of Web pages. It > generally works well. Good to know! I remembered back in the day when full page zoom was first introduced in Firefox, the scolling will be slow as hell when the content is zoomed. That has improved a lot to a point that I have ever set Facebook to zoom -- until I accidentally hit Ctrl+0 one day.

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #49) > Comment on attachment 712479 [details] > PR > > The general approach of this part is correct, but I will strongly against we > land this as-is: > > 0) Various typo and grammar error in comments. Having ambiguous comment is > worse than having no comment. I will fix it, that's an easy thing > 1) Instead of rewriting settings.py (why would you do that?), please pass > the HIDPI parameter to the files, and patch the code respond to the > parameter. Ok, sounds a better solution :D (the rewriting stuff was a legacy solution) > 2) Why would you need to rewrite the Makefile? Same as above > 3) The two checks in webapp-zip.js looks like duplicates from me. I thought > the only thing you want to do in addToZip() is > -- if (HIDPI && this_is_1x_file && the_2x_file_exists ) return; > -- if (!HIDPI && this_is_2x_file && the_1x_file_exists ) return; > right? There are 2 check because of branded shared resources function, it only pass to the zip the linked branded shared resources (instead of scanning all the folder files as the regular function), that means i will never pass an @2x file. So, I'm forcing to pass that branded asset as "@2x" and if it exists will be added to the zip also. > 4) I suppose responsive.css will be included to all apps as shared style. > That works. It will be added in next PR, we want to keep this merging step by step. > 5) I am not sure the 94% in |html { font-size: 94%!important; }| is > intentional or not. 94% of what value you are referring too? Looks to me > this will result the font-size being set to 15.033px, which is 94% of the ua > default (16px). I would recommend setting the value to a literal pixel value. Default ua font-size is 16px sure, and 62.5% of 16px is equal to 10px, that is our baseline. So 62.5% * scale_ratio = wvga 94%, qhd 105% I'm sure this calcs are solid and flexible enough to be used. And also covers the case that concerns Vivien about accessibility. > 6) In the media query, |orientation| should be |device-aspect-ratio|. > |orientation| is by design responsive to the size of the app iframe, not the > real orientation of the phone screen. This has been already fixed, as a suggestion of :rexboy > 7) Please remove the qHD entry in responsive.css, as the setting right now > is a incorrect workaround. for qHD device (540×960, device-pixel-ratio=2), > Gaia should resize to 270x480 (css-)pixels, and have Gecko to map 2 device > pixels to 1 css pixel -- but all that should be addressed in bug 838505, not > here. If we remove the qhd rules we should remove also the wvga ones, as both of them are totally above from our base screen resolution (320x480:163). The bug https://bugzilla.mozilla.org/show_bug.cgi?id=828531 has been already fixed and landed on mc. That means we won't need the whole responsive.css file anymore. In the other way, if you want to keep this file having the rules for qhd does not creates any conflict with wvga ones, and there is no reason to remove it, as both screens are suppose to be supported by Gaia.

Comment on attachment 712479 [details] PR Looks good with comment addressed on Github, thank you!

Merged in master: https://github.com/mozilla-b2g/gaia/commit/cf81c9281538d9d94cbccf3844cc7896eb8ab505

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #50) > Web content (e.g. Gaia), especially web apps, should always be fluid to > device dimensions, and web platform (e.g. Gecko) should be reliable enough > to expose the right parameters to web contents. A qHD device by definition > is 540×960 (physical-)pixels and device-pixel-ratio=2. device-pixel-ratio depends on the physical screen size and viewing distance. As far as I know qHD is just 960x540 and could be any device-pixel-ratio. > What I strongly against was to have Gaia ignore all that differences (and > remove the rem work), and set Gecko pref of device-pixel-ratio to an > arbitrary number (say, 2.232) just because the font look too small. I know > Gecko should works well with the pref, but that's not right. Sizing text in rems makes some sense. Sizing other things like images in rems makes less sense IMHO.

Comment on attachment 712479 [details] PR Already reviewed by Tim :)

(In reply to Jose M. Cantera from comment #44) > (In reply to Chris Jones [:cjones] [:warhammer] from comment #41) > > Generally I want to do whatever roc suggests :). However, we have to ensure > > that pages like games can "see" the real resolution. > > Shouldn't this be controlled by using proper meta viewport directives?, like > target-densitydpi=device-dpi which has been available in Android for a long > time. > > http://developer.android.com/guide/webapps/targeting.html > > There are related discussions in Webkit at > > https://lists.webkit.org/pipermail/webkit-dev/2012-May/020847.html and > related bug at > > https://bugs.webkit.org/show_bug.cgi?id=88047 A related bug is bug 677989

Lockscreen PR merged: https://github.com/mozilla-b2g/gaia/commit/a9e47fcc417ebe102c6aa1bd5de0ebeb9c95fe1b

I'm not sure why PR https://github.com/mozilla-b2g/gaia/pull/8297 was merged - it seems to be causing adverse effects in B2G Desktop http://danheberden.com/share/cf3062b.png - jugglinmike was also able to confirm. How I verified: git reset --hard 91d345428 # problem non-existant git reset --hard a9e47fcc41 # problem introduced Commit: https://github.com/mozilla-b2g/gaia/commit/a9e47fcc417ebe102c6aa1bd5de0ebeb9c95fe1b

Comment on attachment 718447 [details] homescreen PR This r+ is only valid once Rex give his f+.

Comment on attachment 718447 [details] homescreen PR delegating my feedback to Cristian, as he knows perfectly the HS implications. By the way, it would be also nice test that all the subsequent PRs do not break anything in B2G desktop. thanks!

Comment on attachment 718447 [details] homescreen PR My feedback is + but review this comment https://github.com/mozilla-b2g/gaia/pull/8335#discussion_r3167535 that is very important

(In reply to danheberden from comment #60) > I'm not sure why PR https://github.com/mozilla-b2g/gaia/pull/8297 was merged > - it seems to be causing adverse effects in B2G Desktop > http://danheberden.com/share/cf3062b.png - jugglinmike was also able to > confirm. > > How I verified: > git reset --hard 91d345428 # problem non-existant > git reset --hard a9e47fcc41 # problem introduced > > Commit: > https://github.com/mozilla-b2g/gaia/commit/ > a9e47fcc417ebe102c6aa1bd5de0ebeb9c95fe1b That is happening because b2g desktop does not return the proper value for min-device-width, it return computer screen width instead b2g frame. I will open a bug if there is no one open for this issue

Homescreen PR merged: https://github.com/mozilla-b2g/gaia/commit/78afd0dd4e15ced437e1f4c4958977e8e0e2f7e7

(In reply to Ismael from comment #65) > > That is happening because b2g desktop does not return the proper value for > min-device-width, it return computer screen width instead b2g frame. > > I will open a bug if there is no one open for this issue Hmmm I think we can workaround it simply by changing @media all　… to @media handheld　… inside responsive.css to make media query affects only mobile devices, but I haven't tested it yet. Ideally this should work.

Comment on attachment 720759 [details] sms pr SMS_200x200_bubble@2x.png is not updated to the correct size. Do you want to isolate the responsive.css fix and land it first?

Tim i don't know what do you mean about SMS_200x200_bubble@2x.png is not updated to the correct size. Is just twice times bigger than the normal one. About the responsive.css fix, i think if i can get the r+ from sms owners we can land it today. Anyway if the review takes too much time i will land it isolated. Thx!

Comment on attachment 720759 [details] sms pr it was working ok although there is the problem you mention with the keyboard. Please Borja have a look at it before landing this.

Comment on attachment 720759 [details] sms pr Please, address the comment and for me it's R+! ;)

KM Lee, handheld does not work :( so we are going to merge with this dirty patch until there is one better available!

sms&responsive fixes pr merged: https://github.com/mozilla-b2g/gaia/commit/400b9f1752e1dfa8b118f8ca2ebe4549a8cd0471

Hello, I've updated my Gaia, got PR #8444 in, and when looking at the result on Nexus S, I feel the font is way too big. Please find the attached screenshot that exposes this.

Same content, with the full phone picture.

(In reply to Alexandre LISSY :gerard-majax from comment #76) > Created attachment 721172 [details] > SMS screenshot after pull request #8444 > > Hello, > > I've updated my Gaia, got PR #8444 in, and when looking at the result on > Nexus S, I feel the font is way too big. Please find the attached screenshot > that exposes this. This is because the system is not responsive. What we do is scaling up things. The way to approach this is to either fine tune the fonts for nexus S so they look smaller, or make the system full responsive (which i think it's out of scope right now). As far as everything is automatically scaled, it's not possible to reduce the font size as this change would be propagated to the rest of screen resolutions we're supporting right now.

(In reply to Sergi from comment #78) > (In reply to Alexandre LISSY :gerard-majax from comment #76) > > Created attachment 721172 [details] > > SMS screenshot after pull request #8444 > > > > Hello, > > > > I've updated my Gaia, got PR #8444 in, and when looking at the result on > > Nexus S, I feel the font is way too big. Please find the attached screenshot > > that exposes this. > > This is because the system is not responsive. What we do is scaling up > things. The way to approach this is to either fine tune the fonts for nexus > S so they look smaller, or make the system full responsive (which i think > it's out of scope right now). As far as everything is automatically scaled, > it's not possible to reduce the font size as this change would be propagated > to the rest of screen resolutions we're supporting right now. Nexus S is 480x800, it's below qHD of the Peak device. Does this means fonts will be that big on Peak ? Right now, the only solution would be to fix font for Nexus S ?

I'm afraid that's correct. Maybe Ismael can correct me if i'm wrong. Right now we are scaling proportionally based in the font size which is already defined in the CSS. Changing something to fine tune the fonts for the Nexus S (WVGA) will screw up what we have for HVGA and qHD. Anyway i have a Geeksphone Peak with me right now, we've spent some time in adapting visuals, and i don't feel like the fonts are that big.

Find attached a sample picture on how the SMS app looks in a Geeksphone Peak (qHD) and a Unagi (HVGA). Although the aspect ratio is different, and that makes we have more vertical space, fonts are scaled up proportionally based in the base font size defined.

(In reply to Sergi from comment #81) > Created attachment 721296 [details] > SMS :: HVGA vs qHD :: Picture > > Find attached a sample picture on how the SMS app looks in a Geeksphone Peak > (qHD) and a Unagi (HVGA). Although the aspect ratio is different, and that > makes we have more vertical space, fonts are scaled up proportionally based > in the base font size defined. Yep, that's what basiclines explained to me on IRC a couple of hours ago. I still think, even if it might not be the best place for this, that the final rendering makes it too big. I don't remember having the same feeling when seeing Unagi, Otoro or Geeksphone devices.

I did aquick hack testing: $ git diff diff --git a/shared/style/responsive.css b/shared/style/responsive.css index 2bb145e..890f0d1 100644 --- a/shared/style/responsive.css +++ b/shared/style/responsive.css @@ -19,12 +19,12 @@ */ /*Portrait*/ @media all and (min-device-width: 480px) and (max-device-width: 540px) and (max-device-aspect-ratio: 1/1) { - html { font-size: 94%!important; } + html { font-size: 80%!important; } } /*Landscape*/ @media all and (min-device-width: 800px) and (max-device-width: 960px) and (min-device-aspect-ratio: 1/1) { - html { font-size: 94%!important; } + html { font-size: 80%!important; } } I my opinion, it gives a better user experience: fonts are readable, icons are still a bit too big, but it's manageable, and it makes a better use of the screen space available. But it's just my personnal feeling, comparing my daily use on FirefoxOS and Android 2.3 (WVGA too).

(In reply to Ismael from comment #71) > Tim i don't know what do you mean about SMS_200x200_bubble@2x.png is not > updated to the correct size. Is just twice times bigger than the normal one. > My mistake. I was in a hurry... > About the responsive.css fix, i think if i can get the r+ from sms owners we > can land it today. Anyway if the review takes too much time i will land it > isolated. > > Thx! It's fine. Thanks.

Comment on attachment 721635 [details] Browser pr I'll leave this to Ben.

Just caught this bug. Is there a spec you are working against? Reading the comments I can't tell if the following challenges for responsive UI are addressed: 1. The visual design team is investigating various ways to reduce the graphic foot print, ie. putting the action icons into a font rather than keeping them as bitmaps at various sizes. But! To fully support various screen sizes we need 5+ bitmap sizes, with the current 320x480 resolution being the smallest. Are we splitting up gaia into 5+ graphic repos. If not how will a bigger graphic foot print fit onto an Otoro that already is severely space constraint. BTW the rough calculation goes like 2x pixels = 4x bytes. Meaning if all our graphics now are 10MB on a 320x480 device to full support various phone (5) screens like Android it will take up roughly 360MB. 2. We simply can not have the same layout for a 320x480 3.5" @165ppi screen as we would for a 1080x1920 4.8" @ 432ppi screen due to finger reach. On the web responsive websites often have 2-3 different layouts depending on screen size, is this being built in? We shouldn't just enlarge every thing up to the resolution required. When designing for bigger screens there are advantages we need to exploit, like the addition of 1 more row of icons or an extra row of text in a list. 3. Font sizes & touch targets. Currently at 320x480 the default font size for many apps is either 17 or 19px. I really wanted to use physical sizes (mozmm) to optimize consistency and readability. What is our strategy to maintain minimum touch target sizes (atleast 5.5mm) and minimum font sizes (6pt smallest, 8pt default) in the responsive design? Realistically we need to support around 15 layouts in each orientation per app to be responsive. I hope we build that in now, then spend months around v.2 doing this or need to depending on out sourcing all this work.

I am probably misreading this, so please help me understand more clearly :) I'm concerned about the "15 layouts in each orientation per app" part. We have a great deal of layout opportunity since we can use CSS. But perhaps you just mean "layouts" as some rough idea at what a, say, percentage scale on a nav bar would look like at various resolutions?

(In reply to Patryk Adamczyk [:patryk] UX from comment #87) > Just caught this bug. Is there a spec you are working against? Reading the > comments I can't tell if the following challenges for responsive UI are > addressed: I can "understand" that you were not aware of this bug from the work done by Tef (not really, but ok). But there was a lot of effort also from your mates in Taipei about this same issue. So you should have been aware about all this work. > 1. The visual design team is investigating various ways to reduce the > graphic foot print, ie. putting the action icons into a font rather than > keeping them as bitmaps at various sizes. But! To fully support various > screen sizes we need 5+ bitmap sizes, with the current 320x480 resolution > being the smallest. Are we splitting up gaia into 5+ graphic repos. If not > how will a bigger graphic foot print fit onto an Otoro that already is > severely space constraint. BTW the rough calculation goes like 2x pixels = > 4x bytes. Meaning if all our graphics now are 10MB on a 320x480 device to > full support various phone (5) screens like Android it will take up roughly > 360MB. That is not true, we don't need 5 kind of bitmaps, by now we are just using 2, normal ones and HIDPI ones (@2x double sized). Then we scale down the big ones (for HIDPI devices). And yes this is fairly good at visual requirements. Please also summarize these 5 different screens that we have. I only got 3 (320x480, 480x800, 540x960). > 2. We simply can not have the same layout for a 320x480 3.5" @165ppi screen > as we would for a 1080x1920 4.8" @ 432ppi screen due to finger reach. On the > web responsive websites often have 2-3 different layouts depending on screen > size, is this being built in? We shouldn't just enlarge every thing up to > the resolution required. When designing for bigger screens there are > advantages we need to exploit, like the addition of 1 more row of icons or > an extra row of text in a list. I don't think that right now (or ever) exists a screen with that features: "1080x1920 4.8" @ 432ppi" I can imagine you are comparing to Samsung GS4 (not released yet) and is just an example but please use real world examples for real world problems. If we are talking about mobile devices i think we are talking about the same layouts in most cases, i mean just scale up the content for HIDPI devices and if the have an aspect ratio much larger than our base device (like Peak one) just use the extra available vertical space to show up more elements in a list(automatically, no extra effort), homescreen (already done in this Bug), etc.. This bug is aiming to achieve the scale process for mobile devices not to have a responsive system that can work in a variety of devices (tablet layouts, mobile layouts, tv layouts, etc...) Please don't mix it. > 3. Font sizes & touch targets. Currently at 320x480 the default font size > for many apps is either 17 or 19px. I really wanted to use physical sizes > (mozmm) to optimize consistency and readability. What is our strategy to > maintain minimum touch target sizes (atleast 5.5mm) and minimum font sizes > (6pt smallest, 8pt default) in the responsive design? Font-size & touch target are scaled up proportionally, taking unagi,Keon screens type as a base screen. Physical css units (mozmm, pt, etc..) are for physical elements (print media) we should not use ever that units. Just px or rem units. > Realistically we need to support around 15 layouts in each orientation per > app to be responsive. I hope we build that in now, then spend months around > v.2 doing this or need to depending on out sourcing all this work. Please be focus on the Bug title (maybe my branch name is not correct at all responsive-ui), we are just talking about mobile phones layouts.

Comment on attachment 721635 [details] Browser pr I've noticed a small regression with header-divider.png not being displayed correctly in the tab tray at 320x480 resolution. Apart from that I can't spot any obvious regressions caused by this patch so *with that fixed* I can give r+me. I'm a little concerned how future-proof the general approach is here, and using rem and % to size background images is a little concerning too, but I don't want to hold up this whole bug just from the browser patch. As a general comment, we will have a *lot* more than 3 different resolutions to optimise for!

I think we are mixing things here... For me one thing is resolution and the other one is display's screen size. The responsive approach, that means designing different layouts for the same app, makes sense when we have different screen sizes, like for example going from a smartphone to a tablet. In this case it makes sense to have a responsive layout, and load one or the other depending on the device. It's obvious that, having more physical space, it doesn't make sense to just scale things up, but providing the user a different way of doing things taking advantage of the bigger screen size. Another thing is when we have different screen resolution but the screen size is similar. For example when we want to adapt an app from WVGA to qHD. The resolution is different, as well as the pixel density that may also vary, but for me it doesn't make sense to create a responsive UI to go from 960x540px(qHD) to 800x480px(WVGA) taking into account the screen size in inches is will be really similar from one device to the other. What we should present to the user in terms of layout from one smartphone to the other should be the same, but adapting the components in size to fill the device. The responsiveness of the layout should be intelligent enough to know the physical screen size, not the optical resolution. As Ismael pointed out, in the case we have different aspect ratios, and taking into account we are using web technologies, we can scale component's width, and fill the difference in height by displaying more elements. Like it's done in the web. I agree we should think in a responsive approach and that we will have a huge amount of different devices in the future, from smartphones to tablets or TVs, but i'm not sure that's in the scope right now or the purpose of this thread or that it's possible to switch to a responsive approach without investing quite an important amount of time.

Hello Ismael: Since there are per-application patches remaining, may we work together to speed-up the landing process? In specific, I can take some of the apps from your repository, rebase & clean them up, then put them to review. Let me know how do you think about it. Thank you!

(In reply to Ben Francis [:benfrancis] from comment #90) > Comment on attachment 721635 [details] > Browser pr > > I've noticed a small regression with header-divider.png not being displayed > correctly in the tab tray at 320x480 resolution. > > Apart from that I can't spot any obvious regressions caused by this patch so > *with that fixed* I can give r+me. > > I'm a little concerned how future-proof the general approach is here, and > using rem and % to size background images is a little concerning too, but I > don't want to hold up this whole bug just from the browser patch. > > As a general comment, we will have a *lot* more than 3 different resolutions > to optimise for! I will fix that background and merge the PR, thx Ben! (In reply to KM Lee [:rexboy] from comment #92) > Hello Ismael: > > Since there are per-application patches remaining, may we work together to > speed-up the landing process? > In specific, I can take some of the apps from your repository, rebase & > clean them up, then put them to review. Let me know how do you think about > it. Thank you! Sure, i'm happy to hear that!

Browser PR merged: https://github.com/mozilla-b2g/gaia/commit/6136ef06f37ba9785b5ac461e81d9db4f059055f

Shared PR merged: https://github.com/mozilla-b2g/gaia/commit/940124b97032a223a3f618137f8f86b86acc0738

(In reply to Ismael from comment #89) > Physical css units (mozmm, pt, etc..) are for physical elements (print > media) we should not use ever that units. Just px or rem units. pt is just 4/3 of a px. mozmm is very different: assuming everything's implemented properly in Gecko (which it isn't, currently, for FFOS/Gonk), and no zoom, a mozmm is an actual physical millimetre on the screen. So it's a good unit to use for touch targets. Unfortunately it's not standardized, but if app developers think they'd like to have it, that would help me get it standardized :-).

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #96) > (In reply to Ismael from comment #89) > > Physical css units (mozmm, pt, etc..) are for physical elements (print > > media) we should not use ever that units. Just px or rem units. > > pt is just 4/3 of a px. mozmm is very different: assuming everything's > implemented properly in Gecko (which it isn't, currently, for FFOS/Gonk), > and no zoom, a mozmm is an actual physical millimetre on the screen. So it's > a good unit to use for touch targets. Unfortunately it's not standardized, > but if app developers think they'd like to have it, that would help me get > it standardized :-). Sounds really interesting, but people have been complaining a lot about using browser specific functions. I think we should go just the opposite, trying to make the code as much standard and cross-browser as possible

Hey Rex, can you please let me know what feature spec are you working from?

Not sure who is the owner of this system components, if you know him, please add them for reviewers.

Comment on attachment 722732 [details] System-1 PR [statusbar, quick settings, utility tray, notifications, sound manager] Some small nits. Alive, would you review the Volume indicator part?

(In reply to Sergi from comment #97) > (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #96) > > pt is just 4/3 of a px. mozmm is very different: assuming everything's > > implemented properly in Gecko (which it isn't, currently, for FFOS/Gonk), > > and no zoom, a mozmm is an actual physical millimetre on the screen. So it's > > a good unit to use for touch targets. Unfortunately it's not standardized, > > but if app developers think they'd like to have it, that would help me get > > it standardized :-). > > Sounds really interesting, but people have been complaining a lot about > using browser specific functions. I think we should go just the opposite, > trying to make the code as much standard and cross-browser as possible We should use standardized cross-browser features whenever they meet our needs, but sometimes we need features which aren't covered by any existing standard. In that case we create something new and try to get it standardized. In this case I've already discussed mozmm with the CSS WG and the main obstacle to its standardization right now is that no-one's saying they need it.

Comment on attachment 722732 [details] System-1 PR [statusbar, quick settings, utility tray, notifications, sound manager] r- for sound_manager.css I don't understand why the patch ignores all rules about audio channels. It breaks the voice channel volume overlay, please fix.

Comment on attachment 722732 [details] System-1 PR [statusbar, quick settings, utility tray, notifications, sound manager] Fixed telephony data-channel

Comment on attachment 722732 [details] System-1 PR [statusbar, quick settings, utility tray, notifications, sound manager] r+ 'for now': I am going to land https://github.com/mozilla-b2g/gaia/pull/8564 It's a tef+ bug. The main idea is increase the count of segments of telephony 5->6, bt_sco 15->16 So plz make sure you would resolve the conflict then and test your patch again when you are going to merge. I could r+ your patch 'for now'.

(In reply to Patryk Adamczyk [:patryk] UX from comment #98) > Hey Rex, can you please let me know what feature spec are you working from? Hi Patryk: Well, let me try to describe it briefly here. I am now helping Ismael landing his changeset which fixes layout of Gaia applications on high-resolution devices. As far as I know, although we are arguing about underlying solution, most of the layouts are kept similar (on 320x480 device nothing changes), with font and picture enlarged against resolution of different devices. In the changeset there are double-sized picture assets scaled-down to adapt on high resolution devices.

Tidied up from Ismael's repo.

Comment on attachment 724373 [details] Settings PR Everything looks fine, but there are some old changesets that we must review with a Settings app owner. The main point in this PR is removing -moz-image-rect and using background-positions with pseudo-elements. Thx :rexboy

Comment on attachment 724373 [details] Settings PR Evelyn, or any of the Settings app owner can review this.

System-1 PR merged: https://github.com/mozilla-b2g/gaia/commit/daf3928e0ca82a65c79a2c5ba6c2ce6226075ffc

I've noticed that all the UI elements look oversized in B2G Desktop recently, could it be anything to do with the patches in this bug?

(In reply to Ben Francis [:benfrancis] from comment #110) > I've noticed that all the UI elements look oversized in B2G Desktop > recently, could it be anything to do with the patches in this bug? https://bugzilla.mozilla.org/show_bug.cgi?id=845829

After landing commit eaea6311e808ac36cfaea2a74d6a789c68f0863f in gaia master branch the call forwarding icon is no longer shown.

Comment on attachment 725346 [details] System-2 PR Looks good! Thanks.

Did the SMS pr breaks the build and cause bug 851827?

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (PTO Mar 22 - Apr 7) from comment #115) > Did the SMS pr breaks the build and cause bug 851827? I don't know but Ismael can not check it as he's on Holidays next two weeks, perhaps Rexboy can have a look at it.

Comment on attachment 726139 [details] Gallery PR Not giving an r- here, but clearing the review flag because I'm not sure I can actually review it. I don't have a device with a different screen size, which makes it hard to test the proposed changes. Any suggestions on how to try this out? I've only taken a quick glance at the patch, but it looks mostly like dividing pixels by 10 and converting 'px' to 'rem'. That seems mostly harmless. I'm not certain about converting the dimensions of image thumbnails from 106px to 10.6rem, however. On 320x480 devices, the 106px is (320px minus 2px for margins)/3. It divides evenly. On a 480x800 device, the same math does not divide evenly. Does this mean that the thumbnail imags won't be on pixel boundaries? How will that affect their appearance? For the gallery app, I suspect we want everything to be as crisp as we can make it, so I suspect that using resolution-specific media queries may be necessary here, rather than just converting pixels to rems. But I confess that I don't really understand how layout works when using floating-point numbers. Or, if we don't use a media query, maybe we should at least use calc() to compute the thumbnail size rather than using rems. (I think we already have a media query for portait vs landscape mode on the thumbnails.)

:oteo (In reply to Maria Angeles Oteo:oteo from comment #116) > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (PTO Mar 22 - Apr 7) > from comment #115) > > Did the SMS pr breaks the build and cause bug 851827? > > I don't know but Ismael can not check it as he's on Holidays next two weeks, > perhaps Rexboy can have a look at it. OK I will take a look.

(In reply to David Flanagan [:djf] from comment #118) > Comment on attachment 726139 [details] > Gallery PR > > Not giving an r- here, but clearing the review flag because I'm not sure I > can actually review it. I don't have a device with a different screen size, > which makes it hard to test the proposed changes. Any suggestions on how to > try this out? Hi David, thanks for reviewing the patch. Using desktop build may be a way if it is convenient for you. But the media query inside shared/responsive.css should be changed before trying because the queries are just for phone now. For the .thumbnail, It's true that nesting rem inside a media query makes its behavior somewhat unclear. Since the media query is reserved here in css, I think we don't need to stick everything on rem here. I'd try to use px for .thumbnail margin and width then, so that we can solve the floating-point width problem.

Ismael is away so I open another PR to merge for him, with addressed comments by Tim corrected. Carrying r=tim.

Comment on attachment 726139 [details] Gallery PR Updated the code. There are also some update for determining thumbnail size on different resolution devices. For now I am now testing this on both desktop build and S2. When testing on b2g-desktop, I would add "html { font-size: 105%!important; }" in shared/responsive.css in advance with b2g client size set to 960x540. Not sure if there are better way for trying without device. Most of them are just css unit and asset changes though.

Comment on attachment 724373 [details] Settings PR r=me, thanks!

Comment on attachment 724373 [details] Settings PR https://github.com/mozilla-b2g/gaia/commit/7202403530d43a3fbac223f3beef5899faa46665 Settings PR Merged. Thanks for your help, Evelyn!

Comment on attachment 726623 [details] Clock PR Since Rex already fixed the nit from github, it works for me. r+ Nice job!

James may you review this patch? thanks a lot!

Comment on attachment 726623 [details] Clock PR https://github.com/mozilla-b2g/gaia/commit/4072db7b74e4d9677314a73c51ffa849170e8d7c Clock PR merged. Thank you for reviewing Ian!

I believe https://github.com/mozilla-b2g/gaia/commit/0f3a6c729085e322502c05b5dc932bf27ea0139c or https://github.com/mozilla-b2g/gaia/commit/8f5ebdd30c0a41c2fc9e78a3336c7a148d36b35a caused a 100+ ms performance regression on startup of the Settings app. It's now above 1s.

See https://datazilla.mozilla.org/b2g/?branch=master&range=7&test=startup_time&app=settings&app_list=settings&gaia_rev=7202403530d43a3f&gecko_rev=ce5c0b8f9aebdc65 for our metrics.

FM PR

(In reply to Anthony Ricaud (:rik) from comment #129) > I believe > https://github.com/mozilla-b2g/gaia/commit/ > 0f3a6c729085e322502c05b5dc932bf27ea0139c or > https://github.com/mozilla-b2g/gaia/commit/ > 8f5ebdd30c0a41c2fc9e78a3336c7a148d36b35a caused a 100+ ms performance > regression on startup of the Settings app. It's now above 1s. I may found the cause but let's fire another bug for this: Bug 853723

(In reply to KM Lee [:rexboy] from comment #131) > Created attachment 727545 [details] > FM PR > > FM PR @rexboy, I tried your branch (fd-fm) both on the Firefox Nightly with responsive design view (480x800) and the B2G desktop with --screen=480x800, I didn't see any responsive views, is this the right way to check the responsive designs? Please tell me if there is anything that I made a mistake.

(In reply to Pin Zhang [:pzhang] from comment #134) > @rexboy, I tried your branch (fd-fm) both on the Firefox Nightly with > responsive design view (480x800) and the B2G desktop with --screen=480x800, > I didn't see any responsive views, is this the right way to check the > responsive designs? Please tell me if there is anything that I made a > mistake. Pzhang: Oh yes. See comment 122. When using desktop build, the query inspects resolution of our whole monitor, causing it doesn't work as expected. So we need to change a bit of code on resolution.css. Sorry for the confusion. Seems it's better to put a patch for checking purpose. I'll put a patch here.

If you are checking PR on desktop build, apply it with --screen=480x800 so that it sets font-size correctly. As media query matches resolution of whole monitor, we need it to imitate behavior on WVGA phones.

(In reply to KM Lee [:rexboy] from comment #136) > Created attachment 728080 [details] [diff] [review] > Apply before testing on desktop build (for --screen=480x800) > > If you are checking PR on desktop build, apply it with --screen=480x800 so > that it sets font-size correctly. As media query matches resolution of whole > monitor, we need it to imitate behavior on WVGA phones. @rexboy, I applied the patch, yes, it works, but it just enlarged all the images, didn't use the images with @2x that I expected, then what are the @2x images used for?

Pzhang, Try $ HIDPI=1 make The assets are not included in packages unless specifying by HIDPI. Sorry for the confusion again. The usage has been included in README.md at root of repository so you can take a look.

Comment on attachment 728164 [details] Video PR I am happy with this, good job, would be nice to get it merged asap as there is other video work ongoing.

Merged video PR in: https://github.com/mozilla-b2g/gaia/commit/0d589c6991bb4335d69957df3b68596d8aa38964

(In reply to KM Lee [:rexboy] from comment #138) > Pzhang, > Try $ HIDPI=1 make > The assets are not included in packages unless specifying by HIDPI. > Sorry for the confusion again. The usage has been included in README.md at > root of repository so you can take a look. Thanks for your instruction, then the only question that I have is about the decimal precision, please check my comments on the PR page.

(In reply to Pin Zhang [:pzhang] from comment #142) > Thanks for your instruction, then the only question that I have is about the > decimal precision, please check my comments on the PR page. Updated. May you review it again? Thanks a lot!

Comment on attachment 727545 [details] FM PR r=me

(In reply to Pin Zhang [:pzhang] from comment #146) > Comment on attachment 727545 [details] > FM PR > > r=me https://github.com/mozilla-b2g/gaia/commit/2ca952bcca39eda4fa6693ee53cf8c7ec8a3bdcc Merged. Thanks you Pin Zhang!

Comment on attachment 730535 [details] Keyboard PR r=me. Rex, Thanks for your work.

(In reply to Rudy Lu [:rudyl] from comment #150) > Comment on attachment 730535 [details] > Keyboard PR > r=me. > Rex, > Thanks for your work. Keyboard PR merged: https://github.com/mozilla-b2g/gaia/commit/bf0089f663686d03843dbe2e3bec31ea5ed54426 Thank you Rudy!

(In reply to KM Lee [:rexboy] from comment #151) > Created attachment 730617 [details] > Bluetooth PR The pr is relative with Dialer app.

Well.. not sure if this can be tested on desktop build. I can take some screenshot from my S2 if needed. Thanks.

I'm so sorry Ian. It should redirect to the correct place now.

Comment on attachment 731107 [details] Bluetooth PR It works for me. r+

(In reply to Ian Liu [:ianliu] from comment #157) > Comment on attachment 731107 [details] > Bluetooth PR > > It works for me. r+ Merged: https://github.com/mozilla-b2g/gaia/commit/6089f2f0a5698e77494881b58b2721d781c887fc Thank you Ian!

This may not testable on desktop build, so I can record a video here if needed.

This is the one with some javascript changes. Most of them are for adjusting image sizes.

(In reply to KM Lee [:rexboy] (Away 4/4~4/14) from comment #132) > Created attachment 727596 [details] > ftu PR There was some images on FTU that had been change recently. Not sure what would be better, to approve this and open a follow bug, or fix them in here before merging I made some comments on the specific images on github. Apart from that, looks fine to me :)

Adding dev-doc-needed keyword. HIDPI should be in the official docs at least as a flag you can set in .userconfig: https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Customization_with_the_.userconfig_file And probably also in the Gaia docs as well.

Comment on attachment 726139 [details] Gallery PR I don't know much about Window.matchMedia(), but it seems like a pretty heavyweight API to use. How about just checking window.innerWidth and window.innerHeight. I've included some proposed code in my github comments. Otherwise that than, the patch looks good. I haven't tried it, since I don't have a high-resolution device and have forgotten how to use b2g-desktop

Comment on attachment 731824 [details] Wallpaper PR This PR adds new larger wallpaper images, but doesn't have any code that uses them. It it still hardcodes the 320x480 images size, I think. Also, the current code resizes wallpapers in a way that will distort the image when used on a screen with a different aspect ratio. I think we might want to write code that crops the edges so that the aspect ratios match and the image is not distorted. Both of these things are more JavaScript intensive than most of the other patches in this bug, so if you want to spin this off as a separate bug, I'd be happy to give r+ for just the CSS changes here.

Comment on attachment 728164 [details] Video PR Removing the r? flag on me here, since Dale got to this review well before I could.

It seems a problem with the icon scale.

Furthermore, in the widget the spinner rotation is eccentric.

Comment on attachment 731051 [details] Costcontrol PR Due to several regressions in warning icons and spinners when in default screen mode.

Nominating for tef+ because this patch will fix bug 855021 which is tef+

Sigh, that's not going to work because the needed commit (e0d4130a06481c881cbac587bf856f3730b4e505) is dependent on lots of other commits in this bug.

Comment on attachment 727154 [details] Calendar PR ><html> > <head> > <meta http-equiv="Refresh" content="2; > url=https://github.com/mozilla-b2g/gaia/pull/8980"> > </head> > <body> > Redirect to pull request 8980> </body> ></html> >

Calendar version rebased and has my R+ (rexboy still gets commit credit)...

Updated PR addressing Salva comments

Comment on attachment 733304 [details] Wallpaper PR I'm giving r+ on this with these caveats: 1) I haven't tested the code 2) Are you sure you want to remove the pick.js changes? As it stands, the code will only work if the screen size matches the image size exactly. So if you are trying to support more than one large screen size, I don't think this will work. 3) Thanks for the explanation of how the @2x images work. Now that I understand that, I'd prefer a patch that changed resources/320x480/ to resources/images/ so that we are not putting images of varying sizes in a 320x480 directory. But you can defer that to a follow-up patch if you prefer to land this one as is.

Hey David, about the pick.js changes: when the background is placed inside the system app it uses `background-size: cover` to deal with these aspect ratios differences. I mean if you use an image with different aspect ratio that the device one that CSS rule will fill all the screen with the image without deforming it. If this is not working in this way, i will like to have a quick chat with you to be sure that what we are landing is safe enough.

Comment on attachment 731106 [details] Camera PR Merged in: https://github.com/mozilla-b2g/gaia/commit/a268ae3521516a8992fcaf17402ef2cac5ea2bd7

Comment on attachment 733830 [details] FTU PR For the FTU part: - The original for image https://github.com/basiclines/gaia/blob/1040644ae18cfbb0286436d914769269d0d9ee79/apps/communications/ftu/css/images/bg_bubbles%402x.png has recently change (bug 823472) so we need a 2x version for the new one. - Image https://github.com/basiclines/gaia/blob/1040644ae18cfbb0286436d914769269d0d9ee79/apps/communications/ftu/css/images/wifi_sprite.png is no longer used on FTU since bug 811149 landed, so https://github.com/basiclines/gaia/blob/1040644ae18cfbb0286436d914769269d0d9ee79/apps/communications/ftu/css/images/wifi_sprite%402x.png is no needed either. - Image https://github.com/basiclines/gaia/blob/1040644ae18cfbb0286436d914769269d0d9ee79/apps/communications/ftu/css/images/tutorial/3%402x.png doesn't correspond the current tutorial (https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/ftu/css/images/tutorial/3.png) since bug 830380 landed. - Image https://github.com/basiclines/gaia/blob/1040644ae18cfbb0286436d914769269d0d9ee79/apps/communications/ftu/css/images/icon_tour.png would create a regression, as it was changed on bug 822585. Then image https://github.com/basiclines/gaia/blob/1040644ae18cfbb0286436d914769269d0d9ee79/apps/communications/ftu/css/images/icon_tour%402x.png is not longer needed. The rest of the changes seems ok to me :) So please ask me for review once this changes are done Thanks Isma!

Merged in master Wallpaper PR: https://github.com/mozilla-b2g/gaia/commit/15fcfa59b36023cac4b044af1e454f1efede7df1

Comment on attachment 733830 [details] FTU PR Everything looks good to me :) Thanks for the effort Isma!

FTU PR merged: https://github.com/mozilla-b2g/gaia/commit/c0ec827c4137903bac7b1841939dcefce32a5f36

Comment on attachment 733232 [details] Costcontrol PR Eccentric rotations seems to be another problem. Not blocking on this. You have my blessings. The applications looks nice. Thank you very much. Great job guys!

Costcontrol PR merged: https://github.com/mozilla-b2g/gaia/commit/e0c0f9d8fc38ef2b36d16f272979cd98bf9d50a3

Comment on attachment 735093 [details] Calendar PR in master https://github.com/mozilla-b2g/gaia/commit/f98218f66fae3d6cfd635a7469b700824104a8f8

Comment on attachment 736308 [details] Dialer PR Looks perfect to me... and it even works :p -> r+ Thank you very much, Isma.

Dialer PR merged in master: https://github.com/mozilla-b2g/gaia/commit/2b1843511c64d7dda09fa66fc5ff1bae11d90f5f

Comment on attachment 735795 [details] Contacts PR I'm ok with the non-FB related stuff. Let's leave the FB review to Jose Manuel.