Closed Bug 845690 Opened 11 years ago Closed 10 years ago

Support meta viewport in Firefox OS apps

Categories

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

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla30

People

(Reporter: jmcf, Assigned: kats)

References

(Depends on 1 open bug)

Details

(Keywords: dev-doc-needed)

Attachments

(1 file, 1 obsolete file)

Add a warning if we use our implicit meta-viewport tag
3.22 KB, patch
ehsan.akhgari
: review+
vingtetun
: feedback+
Details | Diff | Splinter Review 
As a follow-up of bug 838505, we have concluded that it would be good to support meta viewport tag in Firefox OS apps. That will provide a real benefit to app authors as they could request to the platform different logical viewports that Gecko could emulate conveniently. 

Standards related materials are at http://dev.w3.org/csswg/css-device-adapt/ although the CSS mechanism has not got traction as far as I know. 

On the other hand it seems that meta viewport is currently supported in Firefox Android as per https://developer.mozilla.org/en-US/docs/Mobile/Viewport_meta_tag which emulates the same properties as Safari on mobile.
What's the use case?

Note, the mobilesafari hacks have become a de-facto standard for "mobile" "browser" content, to the point that that content breaks without it.  b2g never had a choice about implementing those.

But waiting to hear the use case before continuing.
Component: General → Style System (CSS)
OS: Mac OS X → All
Product: Boot2Gecko → Core
Hardware: x86 → All
Version: unspecified → Trunk
(In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> What's the use case?

You described a few of them perfectly in https://bugzilla.mozilla.org/show_bug.cgi?id=677989#c0

> 
> Note, the mobilesafari hacks have become a de-facto standard for "mobile"
> "browser" content, to the point that that content breaks without it.  b2g
> never had a choice about implementing those.
> 
> But waiting to hear the use case before continuing.
Depends on: 677989
Huh, how do you determine display sizes for B2G apps without this? Mapping Pixel to Pixel (like width=device-width or initial-scale=1 does, at least theoretically)?
We are using rem units with a root font-size of 10px, and when we need to scale up the UI in some devices we match it via mediaqueries and apply a bigger font-size. Too much manual work :D
Component: Style System (CSS) → General
OS: All → Mac OS X
Product: Core → Boot2Gecko
Hardware: All → x86
Version: Trunk → unspecified
Not sure why with my comment the flags changed...
OS: Mac OS X → All
Hardware: x86 → All
I believe this should work like on the browser because app developers will want to write one app for every platform, as we promise.
(In reply to Jose M. Cantera from comment #2)
> (In reply to Chris Jones [:cjones] [:warhammer] from comment #1)
> > What's the use case?
> 
> You described a few of them perfectly in
> https://bugzilla.mozilla.org/show_bug.cgi?id=677989#c0

Those use-cases are very specialized. Very few apps should care about them.

(In reply to Julien Wajsberg [:julienw] from comment #6)
> I believe this should work like on the browser because app developers will
> want to write one app for every platform, as we promise.

I suppose it might be needed for compatibility for apps written for other platforms, but as far as I know almost all app developers should just use
<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
which is the default behavior for B2G apps anyway.
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #7)
> (In reply to Julien Wajsberg [:julienw] from comment #6)
> > I believe this should work like on the browser because app developers will
> > want to write one app for every platform, as we promise.
> 
> I suppose it might be needed for compatibility for apps written for other
> platforms, but as far as I know almost all app developers should just use
> <meta name="viewport" content="width=device-width, initial-scale=1,
> user-scalable=no">
> which is the default behavior for B2G apps anyway.

Yep as I said by mail I now understand this and I do agree.

(except for the user-scalable=no default: this should be an a11y option for users)
(In reply to Jose M. Cantera from comment #0)
> Standards related materials are at http://dev.w3.org/csswg/css-device-adapt/
> although the CSS mechanism has not got traction as far as I know. 

This is in IE (used widely for Win8 Metro) and (was?) in Opera. Chromium/Blink support is on the way, currently behind a runtime flag (crbug.com/155477). I think that qualifies as traction.
See Also: → @viewport
We should definitely support the meta viewport tag for apps too. In general apps and webpages should be treated the same as much as possible as to enable a smooth transition between them.

So it's just a matter of finding someone that has time to work on this.
Depends on: 940036
It looks like this will happen when AsyncPanZoomController (APZC) is enabled for apps (bug 909877).
Depends on: gaia-apzc
APZC is turned on for apps in B2G 1.3 and up, and yes, we should be supporting meta viewport tags as a result. As a follow-up to what roc said in comment 7, we apply a default meta-viewport tag equivalent to "width=device-width, height=device-height, user-scalable=no" to apps that don't specify one. This is for backwards compatibility only and will eventually be removed. If apps set a meta-viewport tag themselves we will honor it. App authors are strongly encouraged to always use an explicit meta-viewport tag with the properties they want to avoid breakage later.

I'm going to mark this bug fixed for now, but feel free to reopen it if you disagree.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Adding dev-doc-needed: We should document the addition of meta viewport support in 1.3, and the best practice for B2G apps to avoid breakage from the impending change to the default viewport in some future version (comment 14).
Keywords: dev-doc-needed
>  we apply a default meta-viewport tag equivalent to "width=device-width, height=device-height, user-scalable=no" to apps that don't specify one.

Do you throw a warning to the console in that case ?
Not as far as I know. Should we? The console is generally so littered with warnings that I'm hesitant to add more unless there's a good reason.
I think this is worthwhile to add because it's a hack that will be removed in the future.
I'd like too as well. This is a good warning IMO.
Too much things on my plate in January, sorry...
Untested
Assignee: nobody → bugmail.mozilla
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Also since I'm baking the URL into the developer warning, it would be great if the page at https://developer.mozilla.org/en-US/docs/Mozilla/Mobile/Viewport_meta_tag is either updated with this information, or contains a pointer to the new page with the information.
Comment on attachment 8361279 [details] [diff] [review]
Add a warning if we use our implicit meta-viewport tag

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

When I apply this patch and start any app (e.g. dialer, messages, contacts) I get the following output:

E/GeckoConsole( 1073): [JavaScript Warning: "No meta-viewport tag found. Please explicitly specify one to prevent unexpected behavioural changes in future versions. For more https://developer.mozilla.org/en/docs/Mozilla/Mobile/Viewport_meta_tag" {file: "about:blank" line: 0}]

Do you know if it's supposed to be loading about:blank like that?
Attachment #8361279 - Flags: feedback?(21)
I'm just gonna filter out about:blank, seems easy enough.
Attachment #8361279 - Attachment is obsolete: true
Attachment #8361279 - Flags: feedback?(21)
Attachment #8361326 - Flags: review?(mbrubeck)
Comment on attachment 8361326 [details] [diff] [review]
Add a warning if we use our implicit meta-viewport tag

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

I don't feel qualified to review this code.  I'll note, however, that I think this code can execute multiple times for the same page, spamming the log with duplicate messages.  I'm not sure if that will be a problem in practice, or how to avoid it.
Attachment #8361326 - Flags: review?(mbrubeck)
(In reply to Matt Brubeck (:mbrubeck) from comment #26)
> I'm not sure if that will be a problem in practice, or how to avoid it.

Limit log message to origin?
Shouldn't it only run if the viewport tag is modified? Once mViewportType is set to DisplayWidthHeightNoZoom it should take a different branch in the case statement and only come back in here if something changes.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #28)
> Shouldn't it only run if the viewport tag is modified?

For example, TabChild checks the viewport info on every DOMMetaAdded event, even if the <meta> element in question does not have name="viewport".  I think those happen only if a <meta> element is added dynamically (not during initial parsing), so it's probably rare that it's called a lot.  I also don't know whether that code is used in apps.  As I said, I'm not sure if this is a problem in practice.
yeah, I've never seen anyone dynamically adding a meta viewport tag :) I'm not even sure this would work as expected in other browsers (as it's not really spec-ed).
(In reply to Matt Brubeck (:mbrubeck) from comment #29)
> For example, TabChild checks the viewport info on every DOMMetaAdded event,
> even if the <meta> element in question does not have name="viewport".  I
> think those happen only if a <meta> element is added dynamically (not during
> initial parsing), so it's probably rare that it's called a lot.  I also
> don't know whether that code is used in apps.  As I said, I'm not sure if
> this is a problem in practice.

I don't believe this will cause that code to run again. nsDocument::GetViewportInfo will get run again, sure. But mViewportType should be set to !Unknown when that function runs, so it will take the shortcut case statement at [1]. Only if the meta viewport tag is actually changed will the mViewportType get reset back to Unknown at [2] will the code (and log output) run again. And that only happens if the meta viewport tag is modified, in which case I think it's reasonable to print the log statement again (if it triggers, which it probably won't, because there is likely to be a meta viewport tag at that point).

[1] http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp?rev=d470ebf7d063#6866
[2] http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp?rev=d470ebf7d063#3459
Comment on attachment 8361326 [details] [diff] [review]
Add a warning if we use our implicit meta-viewport tag

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

I'm fine with the code but I don't think my r+ is enough to land in content/base/ :)
Attachment #8361326 - Flags: review?(21) → feedback+
Component: General → DOM
Product: Firefox OS → Core
Comment on attachment 8361326 [details] [diff] [review]
Add a warning if we use our implicit meta-viewport tag

That "suggested reviewers" thing is handy! (If the bug is in the right component)
Attachment #8361326 - Flags: review?(jonas)
Note that Jonas comes back from holiday next Monday. Maybe :smaug can have a look?
Smaug, feel free to steal the review if you feel like it. No big rush on this though.
Flags: needinfo?(bugs)
Flags: needinfo?(bugs)
Review ping. This is really a trivial patch.
Attachment #8361326 - Flags: review?(jonas) → review+
Whoops, accidental keyword set.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a85e47332d67
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: