When arguments of viewport's tag are processed, ";" is not recognized as separator as below, static bool isSeparator(UChar c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '=' || c == ',' || c == '\0'; } However, some website is using ";" as separator. For example, in http://m.flickr.com/#/home, it is using ";" for arguments of viewport as below, <meta name="viewport" content="width=device-width; initial-scale=1.0; maximum-scale=1.0; user-scalable=0;"> So, the "device-width;" argument is not processed by processArguments() function because the "deveice-width;" contains ";". In my opinion, we need to add ";" to isSeparator() as below, static bool isSeparator(UChar c) { return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '=' || c == ',' || c == '\0' || c == ';'; } If there are better solutions, please let me know.
Created attachment 57963 [details] patch-1 Please review a patch attached.
After doing some quick research, the Safari documentation[1] clearly demonstrates that multiple tags should be divided by commas instead of semicolons. Other websites, including some Opera's introduction about the viewport meta[2] and a fair amount of other websites describing the possibility, talk solely about dividing by commas as well. The CSS3 Media Queries spec[3], which certainly has some similarities with viewport, does not list a semicolon in its syntax either. Are you sure a change like this is desirable? A much easier solution probably is asking Flickr to fix their meta-tag. [1] http://developer.apple.com/safari/library/documentation/appleapplications/reference/safarihtmlref/articles/metatags.html#//apple_ref/html/const/viewport [2] http://dev.opera.com/articles/view/opera-mobile-9-5-the-developer-angle/ [3] http://www.w3.org/TR/css3-mediaqueries/#syntax
Thank you for your your opinion. It seems I need to investigate this specification further.
(In reply to comment #2) > After doing some quick research, the Safari documentation[1] clearly demonstrates that multiple tags should be divided by commas instead of semicolons. Other websites, including some Opera's introduction about the viewport meta[2] and a fair amount of other websites describing the possibility, talk solely about dividing by commas as well. The CSS3 Media Queries spec[3], which certainly has some similarities with viewport, does not list a semicolon in its syntax either. > > Are you sure a change like this is desirable? A much easier solution probably is asking Flickr to fix their meta-tag. > > [1] http://developer.apple.com/safari/library/documentation/appleapplications/reference/safarihtmlref/articles/metatags.html#//apple_ref/html/const/viewport > [2] http://dev.opera.com/articles/view/opera-mobile-9-5-the-developer-angle/ > [3] http://www.w3.org/TR/css3-mediaqueries/#syntax That's all true, but iphone-4 and android both support comma and semicolon for viewport tag separator. It seems also, that android support space as separator also, but iphone-4 doesn't.
The patch is not quite complete. In addition to modifying isSeparator() it should also modify two lines in the function that calls isSeparator(), Document::processArguments(), to replace if (buffer[i] == ',' || i >= length) with if (buffer[i] == ',' || buffer[i] == ';' || i >= length)
Created attachment 84902 [details] Proposed Patch Thank you for your comment. I add ';' to the two lines. Iphone and Android already support semicolon as separator, I think we can add semicolon to separator. I add comments to this patch.
Android has removed support for ; and iOS does not support it. What iOS (and WebKit trunk now) supports is trailing garbage. ie the ; in width = 230; is considered garbage and thus ignored. This means that width will indeed become 230. Closing as wontfix.
Comment on attachment 84902 [details] Proposed Patch Cleared review? from attachment 84902 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
(In reply to comment #8) > (From update of attachment 84902 [details]) > Cleared review? from attachment 84902 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again). Kenneth said we don't need this patch. Do you think this patch can be needed ?