916185 - Various gaia apps don't set a meta-viewport tag

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

The contacts and dialer apps both don't don't set meta-viewport tags, so they get rendered all zoomed out and with a wide viewport when APZC is turned on. This is probably the case with many other apps as well. I don't know if it's better to fix this on a per-app basis or put in some platform hack to work around it. I would lean towards fixing it on a per-app basis unless there's a compat problem with third-party apps.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Patch contacts and dialer (should change to use , instead of ;)

As an example of what needs to be done

Comment 2

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #0)
> ...I would lean towards fixing it on a per-app basis
> unless there's a compat problem with third-party apps.

+1

Comment 3

[Anthony Ricaud (:rik)]

As we discussed, it's really trivial to add this to every Gaia app. But I'm concerned about this change of behaviour. Right now, 3rd party apps without a meta-viewport tag are rendered with kind of an implicit meta-viewport with width=device-width. We've shipped 3 versions of B2G with this mechanism so I expect changing this will break a lot of 3rd party apps.

I think we should investigate why it was done that way before fixing Gaia apps.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

I was looking around in the code and I didn't find anything that explicitly sets the viewport to be device-width for apps; I think the code was just written that way and nobody bothered to fix it before.

One other thing that came up while I was discussing this with vivien is that we also want the apps to behave the same across all of our platforms. If you install a web app in Fennec, for example, I believe it needs to have a meta-viewport tag or it will render with the same 980-px CSS viewport (I haven't checked that but that is based on my recollection). If we want the app to behave the same on Fennec and Firefox OS then we should force 3rd party apps to add the meta viewport. It might be painful now but it might end up saving more pain in the long run.

Comment 5

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Anthony Ricaud (:rik) from comment #3)
> As we discussed, it's really trivial to add this to every Gaia app. But I'm
> concerned about this change of behaviour. Right now, 3rd party apps without
> a meta-viewport tag are rendered with kind of an implicit meta-viewport with
> width=device-width. We've shipped 3 versions of B2G with this mechanism so I
> expect changing this will break a lot of 3rd party apps.

So, can we add the width=device-width in all of our apps for 1.2, ship it that way, and then start educating the 3rd party developers they should do the same thing.  And tell them that if they don't, things will break in 1.3.  That way we stop the bleeding now, but give them a chance to react and fix it before 1.3.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

I filed bug 940036 specifically to get 3rd party apps updated. We should at least get the word out there as soon as possible.

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Right now, APZ on Gaia apps is blocking for 1.3.  Can we get somebody with Gaia to work with us to understand whether this is a realistic goal or not?

Comment 9

[Milan Sreckovic [:milan] (needinfo for best results)]

Attachment: Calendar, camera, dialer, radio, sms, video (same dialer as in the patch above)

Comment 10

[Jason Smith [:jsmith]]

(In reply to Milan Sreckovic [:milan] from comment #8)
> Right now, APZ on Gaia apps is blocking for 1.3.  Can we get somebody with
> Gaia to work with us to understand whether this is a realistic goal or not?

This is not a committed feature for 1.3 - this should not block.

Comment 11

[Julien Wajsberg [:julienw]]

I'd more agree with Rik in comment 3 here, for several reasons.

* this has been the behavior since the start, we're breaking compatibility
* this is a sensible default for installed apps. This makes sense to say "ok, installed apps are targeting this device anyway"
* I think meta viewport were purely ignored previously. (but I may be wrong)

I think that if any meta viewport is defined, it should be followed. But if no meta viewport is there, a sensible default should be used instead. And that sensible default is probably like "initial-scale=1;user-scalable=no". 

Note that I usually tend to specify only "initial-scale=1" without "width=device-width" as I think "width=device-width" is wrong when in landscape mode (IIUC it should be "width=device-height" in this situation) whereas "initial-scale=1" is always right.

Comment 12

[Julien Wajsberg [:julienw]]

(In reply to (PTO Nov21-Dec01) Kartikaya Gupta (email:[e-mail]) from comment #4)
> I was looking around in the code and I didn't find anything that explicitly
> sets the viewport to be device-width for apps; I think the code was just
> written that way and nobody bothered to fix it before.

see bug 845690 comment 7 from roc, he's actually saying there is a default behavior.

Comment 13

[Milan Sreckovic [:milan] (needinfo for best results)]

(In reply to Julien Wajsberg [:julienw] from comment #12)
> (In reply to (PTO Nov21-Dec01) Kartikaya Gupta (email:[e-mail]) from
> comment #4)
> > I was looking around in the code and I didn't find anything that explicitly
> > sets the viewport to be device-width for apps; I think the code was just
> > written that way and nobody bothered to fix it before.
> 
> see bug 845690 comment 7 from roc, he's actually saying there is a default
> behavior.

As a backgroun, once APZ is on, we need initial-scale=1 or things don't work properly.  In other words, we get different behavior when initial-scale=1 is specified vs. not specified at all.  This makes sense, as defined, if not specified, initial-scale is supposed to be computed based on a bunch of other values.

If our apps are expecting initial-scale=1 behavior, without specifying that value, wouldn't that be wrong?  I don't think we should decide that initial-scale=1 is a good default for B2G apps, where it isn't anywhere else.  It certainly seems like a wrong thing to do.

Or did I misunderstand the issue?

Comment 14

[Julien Wajsberg [:julienw]]

(In reply to Milan Sreckovic [:milan] from comment #13)

> If our apps are expecting initial-scale=1 behavior, without specifying that
> value, wouldn't that be wrong?  I don't think we should decide that
> initial-scale=1 is a good default for B2G apps, where it isn't anywhere
> else.  It certainly seems like a wrong thing to do.

I wouldn't mind one way or the other if we were at the start of the project. But now we have already existing contents that rely on this behavior...

If we want to move away from this behavior, I would suggest to first keep this default, but issue a warning when there is no viewport, communicate about this change, and in a future version (say Firefox OS v1.5, Gecko 34; or Gecko 36) we remove the default.

But see also this mail from :roc in [1] (and maybe the whole thread). I think you should definitely talk together :)

[1] https://groups.google.com/forum/#!msg/mozilla.dev.gaia/BwCdYFLTKi0/MfVt97vStwgJ

Comment 15

[Milan Sreckovic [:milan] (needinfo for best results)]

There is a separate bug 940036 about communicating about this change, and if it goes out now, we have almost four months before 28 ships, and even longer before 1.3 based on 28 is in the hands of the users.  If we're talking about changing a default, from an incorrect, to a correct one, it is better to change it sooner, and while the number of users and applications are smaller.

Whether the default is incorrect or not, is a separate issues, (also covered in this bug), but I wanted to first comment on the "let's give people a whole year to deal with this".

I also don't see why we can't "fix" this now, and continue the conversation about what the default should be.  As far as I can tell, we're mostly discussing what the default behaviour should be when the meta viewport is not specified.  We're not arguing what should happen when it is, correct?  So, if it is specified, it won't hurt us.  It will work today, it will work tomorrow, if or when we change the default, it's guaranteed to be the same (bugs nonwithstanding.)

All we need to figure out now is what the default should be.

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

We should definitely have the same meta-viewport behavior in all contexts. Both apps and webpages. We're aiming to build a single web that behaves consistently.

However I agree existing apps is likely to be a problem :( But let's deal with that over in bug 845690. That way this bug can cover just adding meta viewports in the apps that we support.

Comment 17

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: add.viewport.patch

This patch add a viewport to all html files. If the viewport is not exactly what people wants that fine we can change it later (i will add the script I used right after this patch) but adding a viewport right now will let us try apzc asap.

Comment 18

[Andreas Gal :gal]

Comment on attachment 8337548 [details] [diff] [review]
add.viewport.patch

Review of attachment 8337548 [details] [diff] [review]:
-----------------------------------------------------------------

I am pretty sure we must have a fallback for this or third party apps will break. This patch can't hurt though.

Comment 19

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: get_all_html.sh

Here the script I used to replace everything, just in case we need to change the viewport tag later (or better remove it if there is a nice fallback).

Comment 20

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/3c6ae19b70d6a91e262f645f6da6daf824d8f2f3

Comment 21

[Milan Sreckovic [:milan] (needinfo for best results)]

Naoki, once there is a build with this change, it'd be good to make sure it didn't cause any regressions in the non-APZC scenario.

Comment 22

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Gaia      6415b8b44068596404c10365394544e94edd5ce5
Gecko     http://hg.mozilla.org/mozilla-central/rev/12ea03a70243
BuildID   20131211040203
Version   29.0a1
ro.build.version.incremental=291
Buri

Forgot to mark this as verified.  Verified that APZ on and off for the content.  Will create a test case so that we test this with the APZ on for iframes.