Last Comment Bug 1301854 - CSS with UTF-16LE shown as raw bytes in CSS Rules Inspector
: CSS with UTF-16LE shown as raw bytes in CSS Rules Inspector
Status: VERIFIED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Developer Tools: CSS Rules Inspector (show other bugs)
: 44 Branch
: Unspecified Unspecified
P1 normal (vote)
: Firefox 52
Assigned To: Tom Tromey :tromey
:
: Gabriel Luong [:gl][1 biz day review guarantee] (ΦωΦ)
Mentors:
: 1186268 (view as bug list)
Depends on:
Blocks: 984880
  Show dependency treegraph
 
Reported: 2016-09-09 22:15 PDT by Mehdi
Modified: 2017-02-01 02:17 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard: [testday-20161028]
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
verified

MozReview Requests
Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:
Show discarded requests

Attachments
1.png (8.40 KB, image/png)
2016-09-09 22:15 PDT, Mehdi
no flags Details
bundle.css (136 bytes, text/css)
2016-09-11 06:00 PDT, Loic
no flags Details
index.html (2.76 KB, text/html)
2016-09-11 06:01 PDT, Loic
no flags Details
Bug 1301854 - detect BOM when determining resource encoding; (58 bytes, text/x-review-board-request)
2016-09-16 12:34 PDT, Tom Tromey :tromey
pbrosset: review+
Details | Review

Description User image Mehdi 2016-09-09 22:15:27 PDT
Created attachment 8789972 [details]
1.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160823121617

Steps to reproduce:

The CSS has bundled with UTF-8 unicode.

Please note: other files's CSS shown correctly.


Actual results:

CSS rules shown as decoded characters


Expected results:

Show the correct characters
Comment 1 User image Tooru Fujisawa [:arai] 2016-09-10 01:16:24 PDT
Can you provide a testcase?
Comment 2 User image Mehdi 2016-09-10 21:01:43 PDT
(In reply to Tooru Fujisawa [:arai] from comment #1)
> Can you provide a testcase?

Here you go (https://github.com/dehghani-mehdi/firefox-dev-testcase)
Comment 3 User image Mehdi 2016-09-10 21:02:33 PDT
You can find the testcase, here:
https://github.com/dehghani-mehdi/firefox-dev-testcase
Comment 4 User image Tooru Fujisawa [:arai] 2016-09-11 01:06:04 PDT
thanks :)

the CSS file is using UTF-16LE, and it's shown as raw bytes there.

Here's regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=f354f556b59136bc052e5b2c57d7d28e7242de37&tochange=40f709d9c088dfc4ba1b55fb80920b5ff5102842

looks like it's from bug 984880.
Comment 5 User image Loic 2016-09-11 06:00:48 PDT
Created attachment 8790090 [details]
bundle.css
Comment 6 User image Loic 2016-09-11 06:01:23 PDT
Created attachment 8790091 [details]
index.html
Comment 7 User image Mehdi 2016-09-11 08:14:15 PDT
(In reply to Tooru Fujisawa [:arai] from comment #4)
> thanks :)
> 
> the CSS file is using UTF-16LE, and it's shown as raw bytes there.
> 
> Here's regression range:
> https://hg.mozilla.org/integration/fx-team/
> pushloghtml?fromchange=f354f556b59136bc052e5b2c57d7d28e7242de37&tochange=40f7
> 09d9c088dfc4ba1b55fb80920b5ff5102842
> 
> looks like it's from bug 984880.

NP :]

I'm sorry, but I didn't get your point.
Is this (I call it bug) fixed? or will be fixed?

Other browsers have not any problem with that bundle, incl. Firefox. I saw this problem only in Firefox Dev Edition.
Comment 8 User image Tooru Fujisawa [:arai] 2016-09-11 08:17:30 PDT
(In reply to Mehdi from comment #7)
> Is this (I call it bug) fixed? or will be fixed?

the change in bug 984880 introduced this issue, and the issue is not yet fixed.
Comment 9 User image Tom Tromey :tromey 2016-09-12 07:25:00 PDT
This looks like another instance of bug 118268 - bundle.css has a BOM, but for some
reason the code in StyleSheetActor._getCSSCharset doesn't recognize it.
If I add a charset to the <link>, like <link href="bundle.css" rel="stylesheet" charset="utf-16">,
then it works for me.
Comment 10 User image Patrick Brosset <:pbro> (PTO, back on Feb 27th) 2016-09-16 08:09:07 PDT
This sounds really bad. Triaging as P1. Tom do you think P1 is right for this bug? Is this something that might happen frequently enough? Or is this an edge case?
Comment 11 User image Tom Tromey :tromey 2016-09-16 08:53:31 PDT
(In reply to Patrick Brosset <:pbro> from comment #10)
> This sounds really bad. Triaging as P1. Tom do you think P1 is right for
> this bug? Is this something that might happen frequently enough? Or is this
> an edge case?

I don't know whether it's common to use a BOM or a non-UTF-8 character set for
CSS.  I also don't know whether any data on this is available.  So, I don't know, sorry.
Comment 12 User image Tom Tromey :tromey 2016-09-16 10:46:38 PDT
The bug happens because this code:
https://dxr.mozilla.org/mozilla-central/source/devtools/shared/DevToolsUtils.js#458
... doesn't do any BOM sniffing.
I have a patch.
Comment 13 User image Tom Tromey :tromey 2016-09-16 10:49:31 PDT
*** Bug 1186268 has been marked as a duplicate of this bug. ***
Comment 14 User image Tom Tromey :tromey 2016-09-16 12:34:00 PDT Comment hidden (mozreview-request)
Comment 15 User image Tom Tromey :tromey 2016-09-16 14:12:55 PDT
Oops, I left in some code from an earlier attempt using BinaryInputStream.
Comment 16 User image Tom Tromey :tromey 2016-09-16 14:13:44 PDT
... and attached the wrong version besides.  twofer.
Comment 17 User image Tom Tromey :tromey 2016-09-16 14:14:52 PDT Comment hidden (mozreview-request)
Comment 18 User image Patrick Brosset <:pbro> (PTO, back on Feb 27th) 2016-09-20 04:42:40 PDT
Comment on attachment 8792088 [details]
Bug 1301854 - detect BOM when determining resource encoding;

https://reviewboard.mozilla.org/r/79318/#review78464

::: devtools/client/styleeditor/test/browser.ini:60
(Diff revision 2)
>    !/devtools/client/inspector/shared/test/head.js
>    !/devtools/client/inspector/test/head.js
>    !/devtools/client/inspector/test/shared-head.js
>    !/devtools/client/shared/test/test-actor-registry.js
>    !/devtools/client/shared/test/test-actor.js
> +  utf-16.css

nit: I think we want to keep the !/devtools/client/... lines at the end  of the support-files block. So you can move the utf-16.css line right after the sync.html line.

::: devtools/shared/DevToolsUtils.js:451
(Diff revision 2)
> +      // We do our own BOM sniffing here because there's no convenient
> +      // implementation of the "decode" algorithm
> +      // (https://encoding.spec.whatwg.org/#decode) exposed to JS.
> +      let bomCharset = null;
> +      if (available >= 3 && source.codePointAt(0) == 0xef &&
> +          source.codePointAt(1) == 0xbb && source.codePointAt(2) == 0xbf) {
> +        bomCharset = "UTF-8";
> +        source = source.slice(3);
> +      } else if (available >= 2 && source.codePointAt(0) == 0xfe &&
> +                 source.codePointAt(1) == 0xff) {
> +        bomCharset = "UTF-16BE";
> +        source = source.slice(2);
> +      } else if (available >= 2 && source.codePointAt(0) == 0xff &&
> +                 source.codePointAt(1) == 0xfe) {
> +        bomCharset = "UTF-16LE";
> +        source = source.slice(2);
> +      }

I'll have to trust you on this :) as I have no knowledge whatsoever about how this works.
The code looks Ok to me, and the issue is solved, so I'm willing to R+, but if you think you need some more expert eyes on this piece of code, I suggest asking someone else.
Comment 19 User image Tom Tromey :tromey 2016-09-20 06:39:34 PDT Comment hidden (mozreview-request)
Comment 20 User image Tom Tromey :tromey 2016-09-20 09:10:07 PDT Comment hidden (mozreview-request)
Comment 21 User image Pulsebot 2016-09-20 09:56:57 PDT
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33304c191afb
detect BOM when determining resource encoding; r=pbro
Comment 22 User image Carsten Book [:Tomcat] 2016-09-21 03:07:54 PDT
https://hg.mozilla.org/mozilla-central/rev/33304c191afb
Comment 23 User image Saheda Reza [:Antora] 2016-10-30 13:30:34 PDT
I have successfully reproduced this bug on firefox nightly according to(2016-09-09)

Fixing bug is verified on Latest Nightly--Build ID:(20161030030204),User Agent: Mozilla/5.0 (Windows NT 10.0; rv:52.0) Gecko/20100101 Firefox/52.0

now it is correctly running on latest nightly.

Tested OS-- Windows10 32bit
Comment 24 User image Petruta Rasa [QA] [:petruta] 2016-10-31 03:42:22 PDT
Thanks for verifying, :Antora!

I also checked that this is fixed using latest Nightly 52.0a1 under Mac OS X 10.11. Marking as verified.
Comment 25 User image Mehdi 2017-02-01 02:09:57 PST
Hi,

This issue doesn't fix yet!, why the status is updated to FIXED?
Comment 26 User image Loic 2017-02-01 02:17:59 PST
(In reply to Mehdi from comment #25)
> Hi,
> 
> This issue doesn't fix yet!, why the status is updated to FIXED?

It's fixed in FF52+, read the flags on the right.

Note You need to log in before you can comment on or make changes to this bug.