When formatting minified CSS files don't add line-break in Data uris

RESOLVED FIXED

Status

()

Firefox
Developer Tools: Style Editor
--
trivial
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: shobson, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

3 years ago
When formatting minified CSS files for the style editor, don't add a line break after a semi-colon in a data uri

> url(data:image/png;base64,iVBORw0KGgoA....

Total nitpick. Super minor.

Comment 1

3 years ago
Depends on https://webcompat.com/issues/689
Yes annoying because it leads to wrong analysis. This needs to be fixed.

Where is the code for this?

Updated

3 years ago
Component: Developer Tools → Developer Tools: Style Editor
(In reply to Karl Dubost :karlcow from comment #1)
> Where is the code for this?

https://dxr.mozilla.org/mozilla-central/source/browser/devtools/styleeditor/StyleSheetEditor.jsm#245 looks like a good place to start exploring.. :)
https://dxr.mozilla.org/mozilla-central/source/toolkit/devtools/styleinspector/css-logic.js#991
Very simple algorithm, not surprising that it gets confused by a data: URL semicolon..

Comment 4

3 years ago
Ah thanks Hallvord!
So if I understand the code it prettifies based on characters, and not on tokenizers.
Should there be a preparsing for tokenizing and the doing the unminifying?

Or would something like this work?

```js
  let URL_STATE = false;
  for (let i = 0; i < text.length; i++) {
    let c = text[i];
    if (text.slice(i,i+4) === 'url(') {
      URL_STATE=true;
      }
    if (c === ')' &&  URL_STATE) {
      URL_STATE=false;
      }
    let shouldIndent = false;

    switch (c) {
      case "}":
        // cut for making it clearer
      case ";":
        if !URL_STATE {
          shouldIndent = true;
          }
      case "{":
      // cut for making it clearer
    }
```
Flags: needinfo?(hsteen)

Comment 5

3 years ago
I wonder if `text.slice(i,i+4)` will be too costly.
maybe

    if (c === 'u') {
      if text.slice(i,i+4) === 'url(') {
        URL_STATE=true;
        }
      }

aka do the slice only if it's the 'u' character.
The the code already has some issues that might slow it down a little - regexp test() on a substring() probably is a little expensive too..?

Your code will probably work. I'm a bit worried about the rule for leaving the URL_STATE condition - say, if you find a file with 

/* URLs should start with url( ..*/

potentially big parts of that file would not be wrapped anymore. Also, presumably the string "url" can be in uppercase?
Flags: needinfo?(hsteen)

Comment 7

2 years ago
This has been fixed since bug 1154809.
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.