184444 2018-04-10 01:14:16 -0700 [GTK] webkit_web_view_load_html() garbages linked CSS content 2018-05-02 16:10:51 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Local Build Unspecified Unspecified NEW https://bugs.webkit.org/show_bug.cgi?id=184660 P2 Normal --- 1 mcrha webkit-unassigned bugs-noreply kapouer mcatanzaro oldest_to_newest 1413235 0 337598 mcrha 2018-04-10 01:14:16 -0700 Created attachment 337598 wk2-css.c This looks like bug #127481, but that one is meant to be fixed. Maybe something had been missing. My test with 2.18.6 and git checkout at git-svn-id: http://svn.webkit.org/repository/webkit/trunk@230032 268f45cc-cd09-0410-ab3c-d52691b4dbfc fails. The trick here is to have the HTML content contain UTF-8 letters, it doesn't matter which. Using webkit_web_view_load_bytes (web_view, bytes, NULL, NULL, "file://"); on exactly the same content of loading that with webkit_web_view_load_uri (WEBKIT_WEB_VIEW (wk), "file:///tmp/a.html"); doesn't exhibit the issue, it's only the webkit_web_view_load_html (web_view, html, "file://"); misbehaving. There doesn't change anything whether the HTML content contains <meta http-equiv="content-type" content="text/html; charset=utf-8"> or not, everything is about the content using only ASCII or UTF-8 letters. Attached is my test program, which references webview.css file provided by 'evolution' program/package. The first line contains a command line how to compile and run it. Some more details are shown in the dialog itself. 1413240 1 kapouer 2018-04-10 01:41:38 -0700 Did you try webkit_web_view_load_bytes (web_view, bytes, "text/html", "utf-8", "file://"); ? 1413241 2 mcrha 2018-04-10 02:13:38 -0700 Well, I wrote I did and I wrote it _doesn't exhibit the issue_. Okay, there's a typo which might confuse you, the text: content of loading should be content or loading . I hope it's clearer now. Anyway, from bug #127481, it looks like WebKit does some expectation on file content encoding from parent's file content encoding. I think it's a bad idea. I do not see any reason why I could not have an HTML page in iso-8859-2, which links a CSS file in UTF-8 or even the main document in UTF-16, while the CSS file in UTF-8 or ASCII. Imagine it as a localized HTML pages in various encodings, where all these localized pages share the same CSS file. 1413242 3 mcrha 2018-04-10 02:25:10 -0700 I would go with the comment in WebPageproxy.cpp // FIXME: Get rid of loadHTMLString and just use loadData instead. but it doesn't cover loadAlternateHTMLString(), thus such change would a) require twice as memory (due to copying the content into a GBytes structure), b) it would be incomplete anyway (due to this alternate function). I have a trivial patch for webkit_web_view_load_html() and webkit_web_view_load_plain_text(), if you are interested. 1415443 4 mcatanzaro 2018-04-17 19:55:14 -0700 WebKit doesn't try to guess your file encoding, like Firefox does, because in practice that is probabilistic and going to fail regularly for encodings that matter in practice (e.g. GB-18030). So instead you have to specify it manually if you want something other than the default, which is ISO-8859-1 for web compat. Sadly you cannot feed UTF-8 into WebKit and expect that to work without declaring the encoding. HTML is the easiest case, because HTML allows you to specify the encoding as part of the document. It's when you're trying to use text files that you're really going to have a hard time, as your only options there are (a) HTTP headers (doesn't work for local files), or (b) API request (webkit_web_view_load_bytes()). (In reply to Milan Crha from comment #3) > I would go with the comment in WebPageproxy.cpp > // FIXME: Get rid of loadHTMLString and just use loadData instead. > but it doesn't cover loadAlternateHTMLString(), thus such change would > a) require twice as memory (due to copying the content into a GBytes > structure), > b) it would be incomplete anyway (due to this alternate function). What does this have to do with the bug? > I have a trivial patch for webkit_web_view_load_html() and > webkit_web_view_load_plain_text(), if you are interested. It'd at least be good to see. 1415501 5 338201 mcrha 2018-04-18 01:25:43 -0700 Created attachment 338201 trvial patch (just to show it) (In reply to Michael Catanzaro from comment #4) > WebKit doesn't try to guess your file encoding, ... Maybe I did not use the 'encoding' word properly. If the bug #127481 is right, and it seems it is, then WebKit has some expectation about HTML and its CSS files "encodings", which is wrong, from my point of view. The above test proves it and it's all about HTML and its CSS sub-file. I'll try to rephrase, but I'm afraid it'll not help much. The above wk2-css.c loads an HTML document which contains: <meta http-equiv="content-type" content="text/html; charset=utf-8"> and <link type="text/css" rel="stylesheet" href="file:///usr/.....css"> using webkit_web_view_load_html() function. The webview itself has also set utf-8 as its default encoding. Whether the .css file is loaded properly solely depends on the actual content of the HTML file, which is wrong from my point of view. When the HTML file contains non-ASCII letters, then the .css file is read as UTF-16 (thus it looks like a garbage), when the HTML file contains only ASCII letters, then the .css file is read as UTF-8 (or some other single-byte encoding, it doesn't matter, it's not that important which single-byte encoding it is, because it's only ASCII there). I can mix ASCII HTML with UTF-16 CSS, the same as UTF-8/UTF-16 HTML with ASCII CSS, there should not be any issue with it. Furthermore, I believe most (if not all) UTF-16 files contain the Unicode marker (0xFEFF), thus it's easily detectable that the file is in UTF-16. When the marker is not there, then you can add a bit more heuristic there, but even then I'd expect the CSS is in the default encoding, if no other is passed by the caller. > What does this have to do with the bug? Everything and nothing. The loadData() can specify the encoding which the loadHTML/loadPlainText cannot. And you said you want to be explicit about encodings. > It'd at least be good to see. Here you are. It uses UTF-8 encoding, but it can use the default encoding from the WebKitSettings, it depends whether you'd want to extend the documentation for the two functions too. I do not think the patch is good for production, though, due to the reasons in comment #3. 1415561 6 mcatanzaro 2018-04-18 09:19:31 -0700 Hmmm... gchar* strings are always UTF-8 in GLib APIs, always, that's a GLib convention, not a WebKit convention. So WebKit treats it as such, and the caller is responsible for converting to UTF-8 before calling webkit_web_view_load_html() or webkit_web_view_load_plain_text(). This usually works, because applications can usually assume that files on disk are always UTF-8. But it breaks down badly for HTML and CSS files with WebKit. We have the WebKit API assuming the input is UTF-8 and converts from UTF-8 unconditionally. But then it goes ahead and treats it as ISO-8859-1 by default, or whatever other encoding is specified in WebKitSettings. And WebKit uses UTF-16 for everything internally (WTF::String), so if anything is being treated as UTF-16, that's probably a bug related to a missing conversion somewhere. So while on one hand, it seems natural that input should be UTF-8, it does seem more than a little unfriendly to ignore the default encoding that was set with WebKitSettings. Carlos, what do you think? 1415571 7 mcrha 2018-04-18 09:50:49 -0700 (In reply to Michael Catanzaro from comment #6) > gchar* strings are always UTF-8 in GLib APIs, always, that's a GLib > convention Not general gchar *strings/variables, see for example: https://developer.gnome.org/glib/2.54/glib-Character-Set-Conversion.html#g-filename-to-utf8 But I agree with you, it's a reasonable expectation that gchar * is a single-byte encoding and I'd go even further and expect, in case of webkit_web_view_load_html() and webkit_web_view_load_plain_text() and the one for alternative representation, that the passed-in string is in UTF-8, or better the WebKitSettings' default-encoding. It won't fix the issue of mixed HTML and CSS on its own, but it's a good start. See the mentioned bug #127481 where the guessing on CSS from the actual content of the HTML is possibly done (aka WebPage::loadString() according to the attached patch there, but it is more than 4 years old now, thus it could change heavily meanwhile). 337598 2018-04-10 01:14:16 -0700 2018-04-10 01:14:16 -0700 wk2-css.c wk2-css.c text/plain 3826 mcrha LyogZ2NjIGBwa2ctY29uZmlnIC0tY2ZsYWdzIC0tbGlicyBndGsrLTMuMCB3ZWJraXQyZ3RrLTQu MGAgd2syLWNzcy5jIC1nIC1PMCAtbyB3azItY3NzICYmIC4vd2syLWNzcyAqLwoKI2luY2x1ZGUg PHN0ZGlvLmg+CiNpbmNsdWRlIDxzdGRsaWIuaD4KI2luY2x1ZGUgPHN0cmluZy5oPgojaW5jbHVk ZSA8Z3RrL2d0ay5oPgojaW5jbHVkZSA8d2Via2l0Mi93ZWJraXQyLmg+CgpzdGF0aWMgdm9pZApz aG93X2luc3BlY3Rvcl9jYiAoR3RrV2lkZ2V0ICpidXR0b24sCgkJICAgV2ViS2l0V2ViVmlldyAq d2ViX3ZpZXcpCnsKCXdlYmtpdF93ZWJfaW5zcGVjdG9yX3Nob3cgKHdlYmtpdF93ZWJfdmlld19n ZXRfaW5zcGVjdG9yICh3ZWJfdmlldykpOwp9CgpzdGF0aWMgY29uc3QgZ2NoYXIgKmh0bWxfY29u dGVudCA9CgkiPCFkb2N0eXBlIGh0bWwgcHVibGljIFwiLS8vVzNDLy9EVEQgSFRNTCA0LjAgVFJB TlNJVElPTkFMLy9FTlwiPiIKCSI8aHRtbD4iCgkiPGhlYWQ+IgoJIjxtZXRhIG5hbWU9XCJnZW5l cmF0b3JcIiBjb250ZW50PVwiV0syLUNTUyB0ZXN0XCI+IgoJIjxtZXRhIGh0dHAtZXF1aXY9XCJj b250ZW50LXR5cGVcIiBjb250ZW50PVwidGV4dC9odG1sOyBjaGFyc2V0PXV0Zi04XCI+IgoJIjxs aW5rIHR5cGU9XCJ0ZXh0L2Nzc1wiIHJlbD1cInN0eWxlc2hlZXRcIiBocmVmPVwiZmlsZTovLy91 c3Ivc2hhcmUvZXZvbHV0aW9uL3RoZW1lL3dlYnZpZXcuY3NzXCI+IgoJIjxzdHlsZT4iCgkiLmRl c2NyaXB0aW9uIHsgZm9udC1mYW1pbHk6IG1vbm9zcGFjZTsgZm9udC1zaXplOiAxZW07IH0iCgki PC9zdHlsZT4iCgkiPC9oZWFkPjxib2R5IGNsYXNzPVwiLWUtd2ViLXZpZXctYmFja2dyb3VuZC1j b2xvciAtZS13ZWItdmlldy10ZXh0LWNvbG9yXCI+IgoJIjxkaXYgY2xhc3M9XCJfX2V2by1oaWdo bGlnaHRcIj5oaWdsaWdodGVkIHRleHQgZnJvbSB3ZWJ2aWV3LmNzcywgd2hpY2ggc2hvdWxkIGJl IHB1cnBsZTwvZGl2Pjxicj4iCgkiRXh0cmEgdGV4dCBmcm9tIGFib3ZlIGVudHJ5OiVzICh1c2lu ZyBkaWFjcml0aWNzL3VtbGF1dHMvVVRGLTggbGV0dGVycyBnYXJiYWdlcyB3ZWJ2aWV3LmNzcyBs b2FkLCBidXQgaGF2aW5nIHRoZXJlIEFTQ0lJIG9ubHkgd29ya3MgZmluZSAtIGRvIGNoYW5nZSBp dCBhYm92ZSkiCgkiPC9ib2R5PjwvaHRtbD4iOwoKc3RhdGljIHZvaWQKcmVsb2FkX2h0bWwgKFdl YktpdFdlYlZpZXcgKndlYl92aWV3LAoJICAgICBHdGtFbnRyeSAqZW50cnkpCnsKCWdjaGFyICpo dG1sOwoKCWh0bWwgPSBnX3N0cmR1cF9wcmludGYgKGh0bWxfY29udGVudCwgZ3RrX2VudHJ5X2dl dF90ZXh0IChlbnRyeSkpOwoJd2Via2l0X3dlYl92aWV3X2xvYWRfaHRtbCAod2ViX3ZpZXcsIGh0 bWwsICJmaWxlOi8vIik7CgkvKnsKCQlHQnl0ZXMgKmJ5dGVzOwoJCWJ5dGVzID0gZ19ieXRlc19u ZXdfc3RhdGljIChodG1sLCBzdHJsZW4gKGh0bWwpKTsKCQl3ZWJraXRfd2ViX3ZpZXdfbG9hZF9i eXRlcyAod2ViX3ZpZXcsIGJ5dGVzLCBOVUxMLCBOVUxMLCAiZmlsZTovLyIpOwoJCWdfYnl0ZXNf dW5yZWYgKGJ5dGVzKTsKCX0qLwoJZ19mcmVlIChodG1sKTsKfQoKc3RhdGljIHZvaWQKZW50cnlf Y2hhbmdlZF9jYiAoR3RrRW50cnkgKmVudHJ5LAoJCSAgV2ViS2l0V2ViVmlldyAqd2ViX3ZpZXcp CnsKCXJlbG9hZF9odG1sICh3ZWJfdmlldywgZW50cnkpOwp9CgppbnQgbWFpbiAoaW50IGFyZ2Ms IGNoYXIgKmFyZ3ZbXSkKewoJR3RrV2lkZ2V0ICpkaWFsb2csICpjb250YWluZXIsICp3aywgKmJ0 biwgKmVudDsKCVdlYktpdFNldHRpbmdzICpzZXR0aW5nczsKCglndGtfaW5pdCAoJmFyZ2MsICZh cmd2KTsKCglnX3NldGVudiAoIldFQktJVF9ESVNBQkxFX0NPTVBPU0lUSU5HX01PREUiLCAiMSIs IEZBTFNFKTsKCglkaWFsb2cgPSBndGtfZGlhbG9nX25ld193aXRoX2J1dHRvbnMgKCJ3azItY3Nz IiwgTlVMTCwgMCwgIl9DYW5jZWwiLCBHVEtfUkVTUE9OU0VfQ0FOQ0VMLCBOVUxMKTsKCWd0a193 aW5kb3dfc2V0X2RlZmF1bHRfc2l6ZSAoR1RLX1dJTkRPVyAoZGlhbG9nKSwgMzIwLCA0MDApOwoK CWNvbnRhaW5lciA9IGd0a19kaWFsb2dfZ2V0X2NvbnRlbnRfYXJlYSAoR1RLX0RJQUxPRyAoZGlh bG9nKSk7CgoJYnRuID0gZ3RrX2J1dHRvbl9uZXdfd2l0aF9tbmVtb25pYyAoIl9TaG93IEluc3Bl Y3RvciIpOwoJZ3RrX2NvbnRhaW5lcl9hZGQgKEdUS19DT05UQUlORVIgKGNvbnRhaW5lciksIGJ0 bik7CgoJZW50ID0gZ3RrX2VudHJ5X25ldyAoKTsKCWd0a19lbnRyeV9zZXRfdGV4dCAoR1RLX0VO VFJZIChlbnQpLCAixJvFocSNxZnFvsO9w6HDrcOpIik7CglndGtfY29udGFpbmVyX2FkZCAoR1RL X0NPTlRBSU5FUiAoY29udGFpbmVyKSwgZW50KTsKCgl3ayA9IHdlYmtpdF93ZWJfdmlld19uZXdf d2l0aF9zZXR0aW5ncyAod2Via2l0X3NldHRpbmdzX25ld193aXRoX3NldHRpbmdzICgKCQkiYXV0 by1sb2FkLWltYWdlcyIsIEZBTFNFLAoJCSJkZWZhdWx0LWNoYXJzZXQiLCAidXRmLTgiLAoJCSJl bmFibGUtaHRtbDUtZGF0YWJhc2UiLCBGQUxTRSwKCQkiZW5hYmxlLWRucy1wcmVmZXRjaGluZyIs IEZBTFNFLAoJCSJlbmFibGUtaHRtbDUtbG9jYWwtc3RvcmFnZSIsIEZBTFNFLAoJCSJlbmFibGUt amF2YSIsIEZBTFNFLAoJCSJlbmFibGUtamF2YXNjcmlwdCIsIEZBTFNFLAoJCSJlbmFibGUtb2Zm bGluZS13ZWItYXBwbGljYXRpb24tY2FjaGUiLCBGQUxTRSwKCQkiZW5hYmxlLXBhZ2UtY2FjaGUi LCBGQUxTRSwKCQkiZW5hYmxlLXBsdWdpbnMiLCBGQUxTRSwKCQkiZW5hYmxlLXNtb290aC1zY3Jv bGxpbmciLCBGQUxTRSwKCQkibWVkaWEtcGxheWJhY2stYWxsb3dzLWlubGluZSIsIEZBTFNFLAoJ CU5VTEwpKTsKCgl3ZWJraXRfc2V0dGluZ3Nfc2V0X2VuYWJsZV9kZXZlbG9wZXJfZXh0cmFzICh3 ZWJraXRfd2ViX3ZpZXdfZ2V0X3NldHRpbmdzIChXRUJLSVRfV0VCX1ZJRVcgKHdrKSksIFRSVUUp OwoJd2Via2l0X3dlYl92aWV3X3NldF9lZGl0YWJsZSAoV0VCS0lUX1dFQl9WSUVXICh3ayksIEZB TFNFKTsKCglnX3NpZ25hbF9jb25uZWN0IChidG4sICJjbGlja2VkIiwgR19DQUxMQkFDSyAoc2hv d19pbnNwZWN0b3JfY2IpLCB3ayk7CglnX3NpZ25hbF9jb25uZWN0IChlbnQsICJjaGFuZ2VkIiwg R19DQUxMQkFDSyAoZW50cnlfY2hhbmdlZF9jYiksIHdrKTsKCglzZXR0aW5ncyA9IHdlYmtpdF93 ZWJfdmlld19nZXRfc2V0dGluZ3MgKFdFQktJVF9XRUJfVklFVyAod2spKTsKCgl3ZWJraXRfc2V0 dGluZ3Nfc2V0X2VuYWJsZV9mcmFtZV9mbGF0dGVuaW5nIChzZXR0aW5ncywgVFJVRSk7CgoJZ3Rr X2NvbnRhaW5lcl9hZGQgKEdUS19DT05UQUlORVIgKGNvbnRhaW5lciksIHdrKTsKCWdfb2JqZWN0 X3NldCAoR19PQkpFQ1QgKHdrKSwKCQkiaGV4cGFuZCIsIFRSVUUsCgkJImhhbGlnbiIsIEdUS19B TElHTl9GSUxMLAoJCSJ2ZXhwYW5kIiwgVFJVRSwKCQkidmFsaWduIiwgR1RLX0FMSUdOX0ZJTEws CgkJImhlaWdodC1yZXF1ZXN0IiwgOTAsCgkJIndpZHRoLXJlcXVlc3QiLCA5MCwKCQlOVUxMKTsK CglyZWxvYWRfaHRtbCAoV0VCS0lUX1dFQl9WSUVXICh3ayksIEdUS19FTlRSWSAoZW50KSk7Cgkv KiB3ZWJraXRfd2ViX3ZpZXdfbG9hZF91cmkgKFdFQktJVF9XRUJfVklFVyAod2spLCAiZmlsZTov Ly90bXAvYS5odG1sIik7ICovCgoJZ3RrX3dpZGdldF9zaG93X2FsbCAoZ3RrX2RpYWxvZ19nZXRf Y29udGVudF9hcmVhIChHVEtfRElBTE9HIChkaWFsb2cpKSk7CglndGtfZGlhbG9nX3J1biAoR1RL X0RJQUxPRyAoZGlhbG9nKSk7CgoJZ3RrX3dpZGdldF9kZXN0cm95IChkaWFsb2cpOwoKCXJldHVy biAwOwp9Cg== 338201 2018-04-18 01:25:43 -0700 2018-04-18 01:25:43 -0700 trvial patch (just to show it) wk.patch text/plain 1451 mcrha ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvVUlQcm9jZXNzL0FQSS9nbGliL1dlYktpdFdlYlZp ZXcuY3BwIGIvU291cmNlL1dlYktpdC9VSVByb2Nlc3MvQVBJL2dsaWIvV2ViS2l0V2ViVmlldy5j cHAKaW5kZXggMTEzNTg1ZWEzNy4uNzg2YjkzMmU3ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktp dC9VSVByb2Nlc3MvQVBJL2dsaWIvV2ViS2l0V2ViVmlldy5jcHAKKysrIGIvU291cmNlL1dlYktp dC9VSVByb2Nlc3MvQVBJL2dsaWIvV2ViS2l0V2ViVmlldy5jcHAKQEAgLTI1NzUsMTAgKzI1NzUs MTYgQEAgdm9pZCB3ZWJraXRfd2ViX3ZpZXdfbG9hZF91cmkoV2ViS2l0V2ViVmlldyogd2ViVmll dywgY29uc3QgZ2NoYXIqIHVyaSkKICAqLwogdm9pZCB3ZWJraXRfd2ViX3ZpZXdfbG9hZF9odG1s KFdlYktpdFdlYlZpZXcqIHdlYlZpZXcsIGNvbnN0IGdjaGFyKiBjb250ZW50LCBjb25zdCBnY2hh ciogYmFzZVVSSSkKIHsKKyAgICBHQnl0ZXMgKmJ5dGVzOworCiAgICAgZ19yZXR1cm5faWZfZmFp bChXRUJLSVRfSVNfV0VCX1ZJRVcod2ViVmlldykpOwogICAgIGdfcmV0dXJuX2lmX2ZhaWwoY29u dGVudCk7CiAKLSAgICBnZXRQYWdlKHdlYlZpZXcpLmxvYWRIVE1MU3RyaW5nKFN0cmluZzo6ZnJv bVVURjgoY29udGVudCksIFN0cmluZzo6ZnJvbVVURjgoYmFzZVVSSSkpOworICAgIGJ5dGVzID0g Z19ieXRlc19uZXcoY29udGVudCwgc3RybGVuKGNvbnRlbnQpKTsKKworICAgIHdlYmtpdF93ZWJf dmlld19sb2FkX2J5dGVzKHdlYlZpZXcsIGJ5dGVzLCAidGV4dC9odG1sIiwgTlVMTCwgYmFzZVVS SSk7CisKKyAgICBnX2J5dGVzX3VucmVmKGJ5dGVzKTsKIH0KIAogLyoqCkBAIC0yNjE0LDEwICsy NjIwLDE2IEBAIHZvaWQgd2Via2l0X3dlYl92aWV3X2xvYWRfYWx0ZXJuYXRlX2h0bWwoV2ViS2l0 V2ViVmlldyogd2ViVmlldywgY29uc3QgZ2NoYXIqIGNvCiAgKi8KIHZvaWQgd2Via2l0X3dlYl92 aWV3X2xvYWRfcGxhaW5fdGV4dChXZWJLaXRXZWJWaWV3KiB3ZWJWaWV3LCBjb25zdCBnY2hhciog cGxhaW5UZXh0KQogeworICAgIEdCeXRlcyAqYnl0ZXM7CisKICAgICBnX3JldHVybl9pZl9mYWls KFdFQktJVF9JU19XRUJfVklFVyh3ZWJWaWV3KSk7CiAgICAgZ19yZXR1cm5faWZfZmFpbChwbGFp blRleHQpOwogCi0gICAgZ2V0UGFnZSh3ZWJWaWV3KS5sb2FkUGxhaW5UZXh0U3RyaW5nKFN0cmlu Zzo6ZnJvbVVURjgocGxhaW5UZXh0KSk7CisgICAgYnl0ZXMgPSBnX2J5dGVzX25ldyhwbGFpblRl eHQsIHN0cmxlbihwbGFpblRleHQpKTsKKworICAgIHdlYmtpdF93ZWJfdmlld19sb2FkX2J5dGVz KHdlYlZpZXcsIGJ5dGVzLCAidGV4dC9wbGFpbiIsIE5VTEwsIE5VTEwpOworCisgICAgZ19ieXRl c191bnJlZihieXRlcyk7CiB9CiAKIHN0YXRpYyB2b2lkIHJlbGVhc2VHQnl0ZXModW5zaWduZWQg Y2hhciosIGNvbnN0IHZvaWQqIGJ5dGVzKQo=