CSS Parsing errors are not shown to the user #5

SamHasler · 2015-09-03T14:56:19Z

I get the Unable to load stylesheet undefined with some third party css that uses an @page rule.

The parsing error is an issue with the parser library you're using (NV/CSSOM#5) but not showing the user that there was an error with the parsing is non-helpful.

To be able to see where the error was I edited rasterizeHTML.allinone.js to use the following code:

  c.loadAndInlineCssLinks=function(a,b){
    var c=r(a)
      , e=[];
    console.log("filtered links",c);
    return d.all(c.map(function(a){
      console.log("CSS",a);
      return (q(a,b)
                  .then(
                      function(b){
                        o(a,b.content+"\n")
                                  ,e=e.concat(b.errors)
                      }
                    ,function(ee){
                      console.log("CSS ERROR", [a,ee]);
                      e.push({resourceType:"stylesheet",url:ee.url,msg:"Unable to load stylesheet "+ (ee.url ||a.getAttribute('href')) +" -- "+ee})
                    })
             )}

The text was updated successfully, but these errors were encountered:

cburgmer · 2015-09-03T15:38:35Z

Thanks for digging into the issue.

The reason NV/CSSOM is currently used, is a bug in Chrome (https://code.google.com/p/chromium/issues/detail?id=161644). This might or might not have been fixed.
Also see mention here: https://github.com/cburgmer/rasterizeHTML.js/wiki/Browser-issues#chrome

In case you don't need the respective feature, or target other browsers, you should be able to take out CSSOM of the package, and the code will fallback to the browser's own CSSOM implementation.

SamHasler · 2015-09-04T11:51:13Z

Thanks for the tip on disabling CSSOM. I hadn't realized it would work without it. It actually fixes a rendering issue I had with some gnarly relative positioning, although only in chrome canary; not Chrome stable or Firefox developer edition.

cburgmer · 2015-09-05T05:38:28Z

I'd like to continue ship CSSOM in the allinone build by default, as the Chrome bug fails a basic feature here. Let's hope CSSOM catches up with the newest CSS features.

cburgmer · 2015-09-05T05:39:44Z

But yes, we should make the error reporting better here. The parsing error should be reported.

cburgmer · 2015-09-05T09:09:29Z

I am trying to make the method fail, but can't. Do you have a short example?

I believe it is one of the steps in https://github.com/cburgmer/inlineresources/blob/master/src/inline.js#L118 that is failing.

SamHasler · 2015-09-05T22:15:21Z

From memory:

  @media print {
     @page {
        // rules
      }
  }

Also, HTML parsing errors. E.g.: <namespace:customtag></ namespace:customtag>

(I cut & pasted HTML into a test where our server framework had failed to render a tag.)

I'll check on Monday if there was anything else I has to fix in my code to get the test to run.

cburgmer · 2015-09-06T03:12:26Z

The HTML I can directly reproduce. The @page rule, I cannot, using rasterizeHTML.js/examples/index.html (and the dev console open). Do send me a full example once you have it.

As for the HTML tag issue, I don't have an initial guess where this is happening - it is interesting to see though that there's no direct stack trace.

SamHasler · 2015-09-06T06:09:33Z

I can tell you where the HTML issue is on Monday when I'm back to work.
Same for the @page example.
On Sep 6, 2015 4:12 AM, "Christoph Burgmer" [email protected]
wrote:

The HTML I can directly reproduce. The @page https://github.com/page
rule, I cannot, using rasterizeHTML.js/examples/index.html (and the dev
console open). Do send me a full example once you have it.

As for the HTML tag issue, I don't have an initial guess where this is
happening - it is interesting to see though that there's no direct stack
trace.

—
Reply to this email directly or view it on GitHub
#5 (comment)
.

SamHasler · 2015-09-07T15:57:25Z

looks like the CSS issue may have been relative paths to non-existant images in backround urls. I think @page was a red-herring, I have a test page with it in rendering fine. I'll try and make a reduced testcase tomorrow.

SamHasler · 2015-09-07T16:05:18Z

the html errors are in rasterizeHTML.js isParseError

SamHasler · 2015-09-08T07:58:29Z

I'd forgot to turn CSSOM back on. Here's a testcase:

<!DOCTYPE html>
<html>
<head>
  <meta charset="utf-8">
  <title>@media print { body { @page{} } }</title>
  <style>
    @media print {
      @page {
        body {}
      }
    }
  </style>
</head>
<body>
  @page was parsed.
  (CSSOM won't parse @page, Chrome will)
</body>
</html>

cburgmer · 2015-09-08T13:43:26Z

On the HTML error in rasterizeHTML: This is a limitation, which I've actually documented in the past (should've thought of that earlier). See the last point under https://github.com/cburgmer/rasterizeHTML.js/wiki/Limitations.

The issue is simple: To render any HTML, it needs to be rewritten as valid XML, but the XMLSerializer does not know what to do with those tags, as they are invalid XML as long as now namespace is declared (which could be a temporary work around for you).

If you have an idea how to fix this or a certain expectation on what it should do instead, do open a ticker with rasterizeHTML and we can discuss there.

cburgmer · 2015-09-08T14:08:20Z

Thanks for the test case. I was testing in the wrong browser, and so cssom was never loaded.

For me the error is thrown and visible in the console, and it is unclear to me, why the error is not captured in the promise chain, and properly reported.

However, even if this reporting issue got fixed, we are talking about a non-standard behaviour of the CSS parsing drop-in, as the default behaviour of the browser's seems to be to ignore any invalid CSS and keep on parsing.

The proper solution to this problem to me seems to make Chrome fix the original issue the workaround was implemented for.

SamHasler · 2015-09-08T16:57:01Z

I was trying to create a testcase for our app's currect CSS and so I just
linked straight to the head copy checked out of svn and pasted in a static
copy of a page into the body. Then when I ran the regression test nothing
happened. It took a long while to figure out it was a CSS parsing issue and
longer to work out where in the code to add a console.log to see the error.

Someone else might have concluded that csscritic didn't work and given up
(e.g. me some months ago).

If the errors are reported it allows users to fix their code and get a
working test that's useful to them.
On Sep 8, 2015 3:08 PM, "Christoph Burgmer" [email protected]
wrote:

Thanks for the test case. I was testing in the wrong browser, and so cssom
was never loaded.

For me the error is thrown and visible in the console, and it is unclear
to me, why the error is not captured in the promise chain, and properly
reported.

However, even if this reporting issue got fixed, we are talking about a
non-standard behaviour of the CSS parsing drop-in, as the default behaviour
of the browser's seems to be to ignore any invalid CSS and keep on parsing.

The proper solution to this problem to me seems to make Chrome fix the
original issue the workaround was implemented for.

—
Reply to this email directly or view it on GitHub
#5 (comment)
.

cburgmer · 2015-09-09T07:17:38Z

I think it should be feasible to catch the error, and record properly.

The only issue is though, that it's hard to log the point of the actual failure. I feel JavaScript's error reporting is not making it easy to handle such things across browsers well.

I might find time to look into this, but I'll be happy for any help on this :)

SamHasler · 2015-09-09T11:51:46Z

I've got my css working for now but I'll probably come across it again soon. I'll make a PR if I can work out a way to get the details of the error.

SamHasler · 2015-09-10T08:26:56Z

I've raised a separate issue for the HTML errors: cburgmer/csscritic#65

cburgmer · 2015-10-03T05:41:25Z

Trying to summarise this ticket:

CSSOM in Chrome fails at @page rules.
Those errors are not properly reported.
HTML errors are not properly reported.

I'd like to tackle

via issue Dispose of CSSOM as a dependency #7
will go away when Dispose of CSSOM as a dependency #7 is solved
via issue rasterizeHTML.js DOMParser errors are not reported to the user csscritic#65

Do you think it is OK to close this issue?

SamHasler · 2015-10-03T08:39:00Z

Yep. That sums it up. Ok to close. Good that chrome team picked up that bug.
On Oct 3, 2015 6:41 AM, "Christoph Burgmer" [email protected]
wrote:

Trying to summarise this ticket:

CSSOM in Chrome fails at @page rules.

Those errors are not properly reported.

HTML errors are not properly reported.

I'd like to tackle

via issue Dispose of CSSOM as a dependency #7 Dispose of CSSOM as a dependency #7

will go away when Dispose of CSSOM as a dependency #7
Dispose of CSSOM as a dependency #7 is solved

via issue rasterizeHTML.js DOMParser errors are not reported to the user csscritic#65
rasterizeHTML.js DOMParser errors are not reported to the user csscritic#65

Do you think it is OK to close this issue?

—
Reply to this email directly or view it on GitHub
#5 (comment)
.

cburgmer · 2015-10-03T09:00:25Z

Thanks for prodding the Chrome guys. Helped a lot :)

SamHasler mentioned this issue Sep 3, 2015

Loading CSS in a background tab is flaky #3

Closed

cburgmer mentioned this issue Oct 3, 2015

Dispose of CSSOM as a dependency #7

Closed

cburgmer closed this as completed Oct 3, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CSS Parsing errors are not shown to the user #5

CSS Parsing errors are not shown to the user #5

SamHasler commented Sep 3, 2015

cburgmer commented Sep 3, 2015

SamHasler commented Sep 4, 2015

cburgmer commented Sep 5, 2015

cburgmer commented Sep 5, 2015

cburgmer commented Sep 5, 2015

SamHasler commented Sep 5, 2015

cburgmer commented Sep 6, 2015

SamHasler commented Sep 6, 2015

SamHasler commented Sep 7, 2015

SamHasler commented Sep 7, 2015

SamHasler commented Sep 8, 2015

cburgmer commented Sep 8, 2015

cburgmer commented Sep 8, 2015

SamHasler commented Sep 8, 2015

cburgmer commented Sep 9, 2015

SamHasler commented Sep 9, 2015

SamHasler commented Sep 10, 2015

cburgmer commented Oct 3, 2015

SamHasler commented Oct 3, 2015

cburgmer commented Oct 3, 2015

CSS Parsing errors are not shown to the user #5

CSS Parsing errors are not shown to the user #5

Comments

SamHasler commented Sep 3, 2015

cburgmer commented Sep 3, 2015

SamHasler commented Sep 4, 2015

cburgmer commented Sep 5, 2015

cburgmer commented Sep 5, 2015

cburgmer commented Sep 5, 2015

SamHasler commented Sep 5, 2015

cburgmer commented Sep 6, 2015

SamHasler commented Sep 6, 2015

SamHasler commented Sep 7, 2015

SamHasler commented Sep 7, 2015

SamHasler commented Sep 8, 2015

cburgmer commented Sep 8, 2015

cburgmer commented Sep 8, 2015

SamHasler commented Sep 8, 2015

cburgmer commented Sep 9, 2015

SamHasler commented Sep 9, 2015

SamHasler commented Sep 10, 2015

cburgmer commented Oct 3, 2015

SamHasler commented Oct 3, 2015

cburgmer commented Oct 3, 2015