40204 2010-06-05 02:22:16 -0700 Need to recognize ";" as separator for arguments of viewport tag 2011-03-08 17:24:25 -0800 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) All All RESOLVED WONTFIX P2 Normal --- 1 gyuyoung.kim webkit-unassigned cmarcelo eric kenneth mike.zraly ostap73 peter oldest_to_newest 234581 0 gyuyoung.kim 2010-06-05 02:22:16 -0700 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. 234582 1 57963 gyuyoung.kim 2010-06-05 02:27:05 -0700 Created attachment 57963 patch-1 Please review a patch attached. 234656 2 peter 2010-06-06 06:02:00 -0700 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 234697 3 gyuyoung.kim 2010-06-06 17:16:41 -0700 Thank you for your your opinion. It seems I need to investigate this specification further. 330990 4 ostap73 2011-01-07 13:55:50 -0800 (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. 331789 5 mike.zraly 2011-01-10 11:51:56 -0800 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) 363158 6 84902 gyuyoung.kim 2011-03-06 17:48:52 -0800 Created attachment 84902 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. 363878 7 kenneth 2011-03-08 00:52:02 -0800 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. 364007 8 84902 eric 2011-03-08 09:52:43 -0800 Comment on attachment 84902 Proposed Patch Cleared review? from attachment 84902 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). 364247 9 gyuyoung.kim 2011-03-08 17:24:25 -0800 (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 ? 57963 2010-06-05 02:27:05 -0700 2011-03-06 17:48:52 -0800 patch-1 patch-1.patch text/plain 1347 gyuyoung.kim SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MDc0MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMDYtMDUgIEd5dXlvdW5nIEtpbSAgPGd5dXlvdW5nLmtpbUBz YW1zdW5nLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBOZWVkIHRvIHJlY29nbml6ZSAiOyIgYXMgc2VwYXJhdG9yIGZvciBhcmd1bWVudHMgb2Yg dmlld3BvcnQgdGFnCisKKyAgICAgICAgU29tZSB3ZWJzaXRlcyBhcmUgdXNpbmcgIjsiIGFzIHNl cGFyYXRvci4gU28sICI7IiBhbHNvIG5lZWQgdG8gYmUgCisgICAgICAgIHByb2Nlc3NlZCBhcyBz ZXBhcmF0b3IuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD00MDIwNAorCisgICAgICAgICogZG9tL0RvY3VtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OmlzU2VwYXJhdG9yKToKKwogMjAxMC0wNi0wNSAgQW5kZXJzIEJha2tlbiAgPGFnYmFra2VuQGdt YWlsLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBFcmljIFNlaWRlbC4KSW5kZXg6IFdlYkNv cmUvZG9tL0RvY3VtZW50LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL2RvbS9Eb2N1bWVudC5j cHAJKHJldmlzaW9uIDYwNzQzKQorKysgV2ViQ29yZS9kb20vRG9jdW1lbnQuY3BwCSh3b3JraW5n IGNvcHkpCkBAIC0yMzE4LDcgKzIzMTgsNyBAQCB2b2lkIERvY3VtZW50Ojpwcm9jZXNzSHR0cEVx dWl2KGNvbnN0IFN0CiAvLyBUaG91Z2ggaXNzcGFjZSgpIGNvbnNpZGVycyBcdCBhbmQgXHYgdG8g YmUgd2hpdGVzcGFjZSwgV2luIElFIGRvZXNuJ3QuCiBzdGF0aWMgYm9vbCBpc1NlcGFyYXRvcihV Q2hhciBjKQogewotICAgIHJldHVybiBjID09ICcgJyB8fCBjID09ICdcdCcgfHwgYyA9PSAnXG4n IHx8IGMgPT0gJ1xyJyB8fCBjID09ICc9JyB8fCBjID09ICcsJyB8fCBjID09ICdcMCc7CisgICAg cmV0dXJuIGMgPT0gJyAnIHx8IGMgPT0gJ1x0JyB8fCBjID09ICdcbicgfHwgYyA9PSAnXHInIHx8 IGMgPT0gJz0nIHx8IGMgPT0gJywnIHx8IGMgPT0gJ1wwJyB8fCBjID09ICc7JzsKIH0KIAogdm9p ZCBEb2N1bWVudDo6cHJvY2Vzc0FyZ3VtZW50cyhjb25zdCBTdHJpbmcmIGZlYXR1cmVzLCB2b2lk KiBkYXRhLCBBcmd1bWVudHNDYWxsYmFjayBjYWxsYmFjaykK 84902 2011-03-06 17:48:52 -0800 2011-03-08 09:52:43 -0800 Proposed Patch patch-separator.txt text/plain 2496 gyuyoung.kim ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCAzNzVhNGNlLi41ZWJiMzQwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTcg QEAKKzIwMTEtMDMtMDYgIEd5dXlvdW5nIEtpbSAgPGd5dXlvdW5nLmtpbUBzYW1zdW5nLmNvbT4K KworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBOZWVkIHRv IHJlY29nbml6ZSAiOyIgYXMgc2VwYXJhdG9yIGZvciBhcmd1bWVudHMgb2Ygdmlld3BvcnQgdGFn CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00MDIwNAor CisgICAgICAgIFNvbWUgd2Vic2l0ZXMgYXJlIHVzaW5nIHNlbWljb2xvbiBhcyBzZXBhcmF0b3Iu IFNvLCBzZW1pY29sb24gYWxzbyBuZWVkIHRvIGJlIAorICAgICAgICBwcm9jZXNzZWQgYXMgc2Vw YXJhdG9yLgorCisgICAgICAgICogZG9tL0RvY3VtZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6 OmlzU2VwYXJhdG9yKTogQWRkICc7JyB0byBzZXBhcmF0b3IuCisgICAgICAgIChXZWJDb3JlOjpE b2N1bWVudDo6cHJvY2Vzc0FyZ3VtZW50cyk6CisKIDIwMTEtMDMtMDYgIERhbmllbCBCYXRlcyAg PGRiYXRlc0ByaW0uY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERhcmluIEFkbGVyLgpkaWZm IC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50LmNwcCBiL1NvdXJjZS9XZWJDb3Jl L2RvbS9Eb2N1bWVudC5jcHAKaW5kZXggY2E3NjhkMC4uNjJkMmJiNSAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkNvcmUvZG9tL0RvY3VtZW50LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9kb20vRG9j dW1lbnQuY3BwCkBAIC0yNjQ1LDcgKzI2NDUsMTEgQEAgdm9pZCBEb2N1bWVudDo6cHJvY2Vzc0h0 dHBFcXVpdihjb25zdCBTdHJpbmcmIGVxdWl2LCBjb25zdCBTdHJpbmcmIGNvbnRlbnQpCiAvLyBU aG91Z2ggaXNzcGFjZSgpIGNvbnNpZGVycyBcdCBhbmQgXHYgdG8gYmUgd2hpdGVzcGFjZSwgV2lu IElFIGRvZXNuJ3QuCiBzdGF0aWMgYm9vbCBpc1NlcGFyYXRvcihVQ2hhciBjKQogewotICAgIHJl dHVybiBjID09ICcgJyB8fCBjID09ICdcdCcgfHwgYyA9PSAnXG4nIHx8IGMgPT0gJ1xyJyB8fCBj ID09ICc9JyB8fCBjID09ICcsJyB8fCBjID09ICdcMCc7CisgICAgLy8gRklYTUU6IEFjY29yZGlu ZyB0byBTYWZhcmkncyBkb2N1bWVudCwgc2VtaWNvbG9uIGlzIG5vdCBzZXBhcmF0b3IuIEJ1dCwg ZXhpc3Rpbmcgd2ViIHBhZ2VzIHVzZQorICAgIC8vIGFueSBtaXggb2YgY29tbWFzLCBzZW1pY29s b25zLCBhbmQgc3BhY2VzIGFzIHNlcGFyYXRvcnMuIEZ1cnRoZXJtb3JlLCBJUGhvbmU0IGFuZCBB bmRyb2lkIHN1cHBvcnQKKyAgICAvLyBzZW1pY29sb24gYXMgc2VwYXJhdG9yLiBTbywgc2VtaWNv bG9uIG5lZWRzIHRvIGJlIHJlY29nbml6ZSBhcyBzZXBhcmF0b3IgdW50aWwgdmlld3BvcnQgbWV0 YSB0YWcKKyAgICAvLyBiZWNvbWVzIGEgVzNDIHN0YW5kYXJkLgorICAgIHJldHVybiBjID09ICcg JyB8fCBjID09ICdcdCcgfHwgYyA9PSAnXG4nIHx8IGMgPT0gJ1xyJyB8fCBjID09ICc9JyB8fCBj ID09ICcsJyB8fCBjID09ICc7JyB8fCBjID09ICdcMCc7CiB9CiAKIHZvaWQgRG9jdW1lbnQ6OnBy b2Nlc3NBcmd1bWVudHMoY29uc3QgU3RyaW5nJiBmZWF0dXJlcywgdm9pZCogZGF0YSwgQXJndW1l bnRzQ2FsbGJhY2sgY2FsbGJhY2spCkBAIC0yNjczLDE0ICsyNjc3LDE0IEBAIHZvaWQgRG9jdW1l bnQ6OnByb2Nlc3NBcmd1bWVudHMoY29uc3QgU3RyaW5nJiBmZWF0dXJlcywgdm9pZCogZGF0YSwg QXJndW1lbnRzQ2FsCiAKICAgICAgICAgLy8gc2tpcCB0byBmaXJzdCAnPScsIGJ1dCBkb24ndCBz a2lwIHBhc3QgYSAnLCcgb3IgdGhlIGVuZCBvZiB0aGUgc3RyaW5nCiAgICAgICAgIHdoaWxlIChi dWZmZXJbaV0gIT0gJz0nKSB7Ci0gICAgICAgICAgICBpZiAoYnVmZmVyW2ldID09ICcsJyB8fCBp ID49IGxlbmd0aCkKKyAgICAgICAgICAgIGlmIChidWZmZXJbaV0gPT0gJywnIHx8IGJ1ZmZlcltp XSA9PSAnOycgfHwgaSA+PSBsZW5ndGgpCiAgICAgICAgICAgICAgICAgYnJlYWs7CiAgICAgICAg ICAgICBpKys7CiAgICAgICAgIH0KIAogICAgICAgICAvLyBza2lwIHRvIGZpcnN0IG5vbi1zZXBh cmF0b3IsIGJ1dCBkb24ndCBza2lwIHBhc3QgYSAnLCcgb3IgdGhlIGVuZCBvZiB0aGUgc3RyaW5n CiAgICAgICAgIHdoaWxlIChpc1NlcGFyYXRvcihidWZmZXJbaV0pKSB7Ci0gICAgICAgICAgICBp ZiAoYnVmZmVyW2ldID09ICcsJyB8fCBpID49IGxlbmd0aCkKKyAgICAgICAgICAgIGlmIChidWZm ZXJbaV0gPT0gJywnIHx8IGJ1ZmZlcltpXSA9PSAnOycgfHwgaSA+PSBsZW5ndGgpCiAgICAgICAg ICAgICAgICAgYnJlYWs7CiAgICAgICAgICAgICBpKys7CiAgICAgICAgIH0K