Bug 47607 – meta tag parser needs to support ; as separator due to Android having made that popular

• Component: WebCore Misc.
• Status: RESOLVED INVALID

[Kenneth Rohde Christiansen]

The meta tag parser in Android supports ; in addition to , as separator [1] (see Document::processMetadataSettings(const String& content)) which means that due to the popularity of Android, pages such as Facebook now uses ';' to separate the viewport meta arguments instead of ',', which results in the viewport algorithm computing a wrong viewport (like for instance a width of 1800 pixels instead of 320).

[1] https://android.git.kernel.org/?p=platform/external/webkit.git;a=blob;f=WebCore/dom/Document.cpp;h=1f8edeb3505ce7b2e49ff2b88f2ef92c62d9a68d;hb=HEAD

[Kenneth Rohde Christiansen]

Created attachment 70644 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 70644 [details]
Patch

I'm not saying that I'm necessarily against this change, but it needs a better explanation. Does this work in IE? In Firefox? Opera Mobile? Are these pages (which ones?) broken on iPhone? What do relevant specs say?

What does Android version history say about reasons for such deviation from webkit.org?

I'm also unsure about what the scope of this change is. ChangeLog says "meta tag parser", but it seems that code changes only affect meta viewport. In the latter is the case, perhaps the question about IE and Firefox isn't valid, but other ones still stand.

R- because of all the questions, and the lack of test case or an explanation why one isn't possible.

[Kenneth Rohde Christiansen]

(In reply to comment #2)
> (From update of attachment 70644 [details])
> I'm not saying that I'm necessarily against this change, but it needs a better explanation. Does this work in IE? In Firefox? Opera Mobile? Are these pages (which ones?) broken on iPhone? What do relevant specs say?

Facebook recently changed and I do not have an iPhone with me right now to test, but the site might be doing UA sniffing. On the other hand, I have ran into this problem at least twice before.

> What does Android version history say about reasons for such deviation from webkit.org?

The webkit.org code is upstreamed by RIM, presumable from the iPhone code dump, after Android started shipping this code. I do not know the original reason for this change.

> I'm also unsure about what the scope of this change is. ChangeLog says "meta tag parser", but it seems that code changes only affect meta viewport. In the latter is the case, perhaps the question about IE and Firefox isn't valid, but other ones still stand.

I'm not sure whether any shipped Opera mobile supports viewport meta tag or not, but at least Firefox support is pretty preliminary and IE does not support it.

> R- because of all the questions, and the lack of test case or an explanation why one isn't possible.

I just pushed the patch to get feedback. I believe that some of the viewport meta tests that we are not passing might be due to this issue. These tests come from Opera.

[Antonio Gomes]

Grace, could help cc'ing the proper Android guys, herE?

[Kenneth Rohde Christiansen]

Facebook, at least with Android UA, is using ; as separator.

[Grace Kloba]

(In reply to comment #4)
> Grace, could help cc'ing the proper Android guys, herE?

Here is some history. In Android's original implementation, we didn't support semi-colon as separator. But we got bugs complaining some sites working on iPhone, but doesn't work on Android. So we added semi-colon support in Cupcake.

But I believe we have removed this as the version in WebKit doesn't support it. Can you double check with Froyo port?

[Kenneth Rohde Christiansen]

(In reply to comment #6)
> (In reply to comment #4)
> > Grace, could help cc'ing the proper Android guys, herE?
> 
> Here is some history. In Android's original implementation, we didn't support semi-colon as separator. But we got bugs complaining some sites working on iPhone, but doesn't work on Android. So we added semi-colon support in Cupcake.
> 
> But I believe we have removed this as the version in WebKit doesn't support it. Can you double check with Froyo port?

Following the links for Froyo I end up with the below link, still containing the ";" support.

http://android.git.kernel.org/?p=platform/external/webkit.git;a=blob_plain;f=WebCore/dom/Document.cpp;hb=HEAD

or 

http://android.git.kernel.org/?p=platform/external/webkit.git;a=blob;f=WebCore/dom/Document.cpp;h=1f8edeb3505ce7b2e49ff2b88f2ef92c62d9a68d;hb=0433734318551aac3b7b82b9d8ab5d485d7d0439

[Grace Kloba]

Hmm, the internal change I referred to is made in May. Maybe it is after we cut the Froyo tree. So it should show up in the next public drop.

[Kenneth Rohde Christiansen]

Closing as invalid then!