Closed
Bug 845690
Opened 12 years ago
Closed 11 years ago
Support meta viewport in Firefox OS apps
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: jmcf, Assigned: kats)
References
(Depends on 1 open bug)
Details
(Keywords: dev-doc-needed)
Attachments
(1 file, 1 obsolete file)
(deleted),
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.
Updated•12 years ago
|
Component: General → Style System (CSS)
OS: Mac OS X → All
Product: Boot2Gecko → Core
Hardware: x86 → All
Version: unspecified → Trunk
Reporter | ||
Comment 2•12 years ago
|
||
(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
Comment 3•12 years ago
|
||
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)?
Comment 4•12 years ago
|
||
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
Comment 5•12 years ago
|
||
Not sure why with my comment the flags changed...
OS: Mac OS X → All
Hardware: x86 → All
Comment 6•12 years ago
|
||
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.
Comment 8•12 years ago
|
||
(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)
Updated•12 years ago
|
Blocks: 830306, native-viewport
Comment 9•11 years ago
|
||
(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.
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.
Comment 11•11 years ago
|
||
see also bug 916185
Comment 13•11 years ago
|
||
It looks like this will happen when AsyncPanZoomController (APZC) is enabled for apps (bug 909877).
Depends on: gaia-apzc
Updated•11 years ago
|
Depends on: gaia-apzc-2
Updated•11 years ago
|
No longer depends on: gaia-apzc-2
Assignee | ||
Comment 14•11 years ago
|
||
dev-doc-info |
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: 11 years ago
Resolution: --- → FIXED
Comment 15•11 years ago
|
||
dev-doc-info |
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
Comment 16•11 years ago
|
||
> 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 ?
Assignee | ||
Comment 17•11 years ago
|
||
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.
Comment 18•11 years ago
|
||
I think this is worthwhile to add because it's a hack that will be removed in the future.
Comment 19•11 years ago
|
||
I'd like too as well. This is a good warning IMO.
Assignee | ||
Comment 20•11 years ago
|
||
Either of you care to write the patch? The warning needs to go at http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDocument.cpp?rev=188d1e255d40#6913
Comment 21•11 years ago
|
||
Too much things on my plate in January, sorry...
Assignee | ||
Comment 22•11 years ago
|
||
Untested
Assignee: nobody → bugmail.mozilla
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 23•11 years ago
|
||
dev-doc-info |
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.
Assignee | ||
Comment 24•11 years ago
|
||
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)
Assignee | ||
Comment 25•11 years ago
|
||
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 26•11 years ago
|
||
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)
Comment 27•11 years ago
|
||
(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?
Assignee | ||
Comment 28•11 years ago
|
||
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.
Comment 29•11 years ago
|
||
(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.
Comment 30•11 years ago
|
||
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).
Assignee | ||
Comment 31•11 years ago
|
||
(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
Assignee | ||
Updated•11 years ago
|
Attachment #8361326 -
Flags: review?(21)
Comment 32•11 years ago
|
||
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+
Assignee | ||
Updated•11 years ago
|
Component: General → DOM
Product: Firefox OS → Core
Assignee | ||
Comment 33•11 years ago
|
||
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)
Comment 34•11 years ago
|
||
Note that Jonas comes back from holiday next Monday. Maybe :smaug can have a look?
Assignee | ||
Comment 35•11 years ago
|
||
Smaug, feel free to steal the review if you feel like it. No big rush on this though.
Flags: needinfo?(bugs)
Updated•11 years ago
|
Flags: needinfo?(bugs)
Assignee | ||
Comment 36•11 years ago
|
||
Review ping. This is really a trivial patch.
Updated•11 years ago
|
Attachment #8361326 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 37•11 years ago
|
||
Keywords: checkin-needed
Comment 39•11 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 11 years ago → 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Comment 40•11 years ago
|
||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•