Updating the content for a viewport meta tag should discard old values
Categories
(Core :: DOM: Core & HTML, enhancement, P2)
Tracking
()
People
(Reporter: hiro, Assigned: hiro)
References
(Blocks 1 open bug)
Details
(Whiteboard: [webcompat:p1][needs-wpt-?])
Attachments
(6 files)
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1498729 - Store each viewport meta data by the viewport meta tag and use the last one. r?botond!
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review | |
47 bytes,
text/x-phabricator-request
|
Details | Review |
Assignee | ||
Comment 1•6 years ago
|
||
Updated•6 years ago
|
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.
Comment 3•6 years ago
|
||
See bug 1547409. Migrating whiteboard priority tags to program flags.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 5•5 years ago
|
||
For testing reference, there isn't any web platform test currently checking this (as expected). I also don't see any clear tests in the WebKit or Blink suites which modify an existing meta viewport or add an extra one using regular DOM methods (for instance using setAttribute on an existing tag or using appendChild to add a second one, among other methods).
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
Moving webcompat links for invisionapp.com from bug 1552713.
mozilla.invisionapp.com replaces the content of the same meta element, the initial content is 'width=device-width, initial-scale=1, user-scalable=no, maximum-scale=1, minimum-scale=1, viewport-fit=cover' and then it's replaced by 'width=1080'.
Assignee | ||
Comment 7•5 years ago
|
||
Hsin-Yi, has someone in your team started working on this? If not, I can take this. There seems more sites affected by this than I thought in the wild.
Assignee | ||
Comment 8•5 years ago
|
||
(In reply to Thomas Wisniewski [:twisniewski] from comment #5)
For testing reference, there isn't any web platform test currently checking this (as expected). I also don't see any clear tests in the WebKit or Blink suites which modify an existing meta viewport or add an extra one using regular DOM methods (for instance using setAttribute on an existing tag or using appendChild to add a second one, among other methods).
Unfortunately as of now there is no way to write web platform tests for those cases because there is no web-exposed API to get the result of meta viewport contents that are used for rendering. If we have a way to add web platform tests which run only mobile environments, we can have reftests there.
Comment 9•5 years ago
|
||
Hi :hiro, I'm putting this in our team's working items after a security bug and an ongoing Fission item. This is the next thing but we haven't got a chance to start. It's appreciated you can help. Go ahead, thank you!!!
Assignee | ||
Comment 10•5 years ago
|
||
Moved https://webcompat.com/issues/16851 to bug 1423013 since I was misunderstanding that the content is scaled down on Chrome, but actually it's not.
Assignee | ||
Comment 11•5 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=504e0f924da4b880d2ad893353991cb430526da5(In reply to Hiroyuki Ikezoe (:hiro) from comment #1)
(In reply to Hiroyuki Ikezoe (:hiro) from comment #0)
it seems the new
definition would require processing all contents data given by all viewport
meta tags, so we should keep the merging behavior at this moment in this
bug.
We shouldn't merge since as Bokan said Chrome and Safari use the last one.
Assignee | ||
Comment 12•5 years ago
|
||
The test is supposed that the document is not scalable at all when
"minimum-scale=maximum-scale" is set in the viewport meta content, but it's
actually invalid. A correct content is assumed "minimum-scale=1,maximum-scale=1"
so that the document is not scaled at all.
The reason why this check has passed is that "user-scalable=no" which is set
right before this check remains in cache so that the content of the meta
viewport becomes "user-scalable=no,minimum-scale=maximum-scale" which means the
document is not scalable. But we are going to fix this weird behavior in this
commit series so that we are no longer able to rely on the cached
"user-scalable=no" value.
Assignee | ||
Comment 13•5 years ago
|
||
Depends on D38919
Assignee | ||
Comment 14•5 years ago
|
||
Depends on D38920
Assignee | ||
Comment 15•5 years ago
|
||
This is what Chrome and Safari do.
See https://webcompat.com/issues/20701#issuecomment-436054739
So for exmaple, if there are two viewport meta tags like this;
<meta name="viewport" content="width=980">
<meta name="viewport" content="initial-scale=1,maximum-scale=1">
We will use "initial-scale=1,maximum-scale=1". Before this change we used to
use merged "width=980,initial-scale=1,maximum-scale=1".
Another example is to replace the content of a single viewport meta tag like this;
<meta id="viewport" name="viewport" content="width=device-width, initial-scale=1">
what will happen when this tag is replaced by below;
<meta id="viewport" name="viewport" content="width=1080">
We will use the replacing one (i.e. "width=1080"), before this change, we used
to use merged "width=1080,initial-scale=1".
As of this commit, we don't properly remove corresponding viewport meta data
when a) viewport meta tag is detached from document and b) name
attribute is
changed from 'viewport'. These cases will be handled in subsequent commits.
Note that we no longer store invididual viewport meta data in Document::mHeaderData
so that nsIDOMWindowUtils.getDocumentMetadata doesn't work any more for the
invididual viewport meta data, but there is no use cases for them other than
two test cases which are removed in this commit.
Depends on D38921
Assignee | ||
Comment 16•5 years ago
|
||
Depends on D38922
Assignee | ||
Comment 17•5 years ago
|
||
Depends on D38923
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Assignee | ||
Comment 18•5 years ago
|
||
Note that I had to remove review requests to Olli from commit messages to upload new changesets, but it's still there on the phabricator.
Assignee | ||
Comment 19•5 years ago
|
||
(In reply to Hiroyuki Ikezoe (:hiro) from comment #18)
Note that I had to remove review requests to Olli from commit messages to upload new changesets, but it's still there on the phabricator.
Because he is on PTO. :)
Updated•5 years ago
|
Updated•5 years ago
|
Comment 20•5 years ago
|
||
Comment 21•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/389b7ad24599
https://hg.mozilla.org/mozilla-central/rev/42cff84b0fe4
https://hg.mozilla.org/mozilla-central/rev/06d668fc6814
https://hg.mozilla.org/mozilla-central/rev/07d4c38f5cf9
https://hg.mozilla.org/mozilla-central/rev/f6cede8edb98
https://hg.mozilla.org/mozilla-central/rev/2fdb5eadfb82
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Comment 23•5 years ago
|
||
Not sure if we should consider an esr68 uplift here. Looking at the patches, a rebase to the esr68 branch would be nontrivial, but probably doable if there is a pressing user need.
Comment 24•5 years ago
|
||
Seems like a lot to uplift for a nearly-EOL product. Let's let this fix ride with Fenix.
Description
•