CSS with UTF-16LE shown as raw bytes in CSS Rules Inspector

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: CSS Rules Inspector
P1
normal
VERIFIED FIXED
8 months ago
3 months ago

People

(Reporter: Mehdi, Assigned: tromey)

Tracking

({regression})

44 Branch
Firefox 52
regression
Points:
---

Firefox Tracking Flags

(firefox52 verified)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments, 2 obsolete attachments)

8.40 KB, image/png
Details
58 bytes, text/x-review-board-request
pbro
: review+
Details | Review
(Reporter)

Description

8 months ago
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
Can you provide a testcase?
Flags: needinfo?(dehghani.m.c)

Updated

8 months ago
Keywords: testcase-wanted
(Reporter)

Comment 2

8 months ago
(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)
(Reporter)

Comment 3

8 months ago
You can find the testcase, here:
https://github.com/dehghani-mehdi/firefox-dev-testcase
Flags: needinfo?(dehghani.m.c)
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.
Blocks: 984880
Status: UNCONFIRMED → NEW
Component: Untriaged → Developer Tools: CSS Rules Inspector
Ever confirmed: true
Keywords: testcase-wanted → regression
Summary: [Firefox Developer edition] CSS shown as decoded characters → CSS with UTF-16LE shown as raw bytes in CSS Rules Inspector

Comment 5

8 months ago
Created attachment 8790090 [details]
bundle.css

Comment 6

8 months ago
Created attachment 8790091 [details]
index.html

Updated

8 months ago
Attachment #8790090 - Attachment is obsolete: true

Updated

8 months ago
Attachment #8790091 - Attachment is obsolete: true
(Reporter)

Comment 7

8 months ago
(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.
(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.
(Assignee)

Comment 9

8 months ago
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.
See Also: → bug 118268
(Assignee)

Updated

8 months ago
See Also: → bug 1186268
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?
Flags: needinfo?(ttromey)
Priority: -- → P1
(Assignee)

Comment 11

7 months ago
(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.
Flags: needinfo?(ttromey)
(Assignee)

Updated

7 months ago
See Also: bug 118268
(Assignee)

Comment 12

7 months ago
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.
Assignee: nobody → ttromey
Status: NEW → ASSIGNED
(Assignee)

Updated

7 months ago
Duplicate of this bug: 1186268
Comment hidden (mozreview-request)
(Assignee)

Comment 15

7 months ago
Oops, I left in some code from an earlier attempt using BinaryInputStream.
(Assignee)

Comment 16

7 months ago
... and attached the wrong version besides.  twofer.
Comment hidden (mozreview-request)

Comment 18

7 months ago
mozreview-review
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.
Attachment #8792088 - Flags: review?(pbrosset) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 21

7 months ago
Pushed by ttromey@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/33304c191afb
detect BOM when determining resource encoding; r=pbro

Comment 22

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/33304c191afb
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52
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
QA Whiteboard: [testday-20161028]
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.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
(Reporter)

Comment 25

3 months ago
Hi,

This issue doesn't fix yet!, why the status is updated to FIXED?
Version: 50 Branch → 51 Branch

Comment 26

3 months ago
(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.
Version: 51 Branch → 44 Branch
You need to log in before you can comment on or make changes to this bug.