chrome.tabs.insertCSS does not give feedback about CSS syntax errors
Reported by
d...@adblockplus.org,
Mar 23 2017
|
||||||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36
Steps to reproduce the problem:
1. Open a tab, find it's ID.
2. Open the background console for any Chrome extension and run this (replacing 42 with the tab ID):
chrome.tabs.insertCSS(
42,
{code: "*, foo[bar=\"1] {display: none}"},
console.log
)
What is the expected behavior?
An exception should be thrown, or the callback should be passed a flag to say the CSS inserted had a syntax error. Since the attribute selector is missing the closing quote the rule will not be applied at all.
What went wrong?
The callback is passed nothing, no exception is thrown and nothing is hidden due to the syntax error.
Did this work before? N/A
Does this work in other browsers? N/A
Chrome version: 56.0.2924.87 Channel: stable
OS Version: Debian
Flash Version: N/A
When Adblock Plus switches to use chrome.tabs.insertCSS it would be very useful to know when the inserted CSS rule has a syntax error.
1. For element hiding filters we combine many many CSS selectors into one "display:none" CSS rule, inserting a rule for each selector separately would be slow.
2. Sometimes filters can have mistakes which result in a syntactically incorrect selector.
3. When the rule containing an broken selector is inserted it has no effect.
4. At that point we need to know, so we can go back and insert a rule separately for each selector of the broken batch.
See related discussion https://issues.adblockplus.org/ticket/5015
,
Mar 30 2017
,
Mar 30 2017
Removing Blink>CSS - this is a feature request for extensions.
,
Mar 31 2017
This is a pretty reasonable idea. Extensions code uses WebDocument::insertStyleSheet [1] in order to add css sheets; is there any way we could get an error from that in the case of a syntax error? (It looks like the sheet is parsed synchronously, but I don't know for sure.) pdr@, do you know who works on CSS nowadays that would know this? [1] https://chromium.googlesource.com/chromium/src/+/3f4231f979090ad23fd89622b3b36ed564650645/third_party/WebKit/Source/web/WebDocument.cpp#185
,
Apr 5 2017
,
Apr 6 2017
When we have lazy parsing, it'd be pretty hard to know that in general. csharrison@ might know more.
,
Apr 6 2017
As lazy parsing stands today, we don't support lazy parsing things passed into parseString. Agreed that in general this would be difficult, but for certain parsing code paths it shouldn't be too hard to turn the optimization off.
,
Apr 6 2017
Would you want to turn off the optimisation for extensions though? I don't know they'd have that much less of a problem with unused rules. Though, even without lazy parsing there'd be a ton of plumbing required for this...
,
Apr 6 2017
The two features are pretty much mutually exclusive (as far as I can tell), so I think this comes down to a tradeoff between speed and ergonomics.
,
Apr 6 2018
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue. Sorry for the inconvenience if the bug really should have been left as Available. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 6 2018
Still potentially desirable, but pretty low priority. |
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by ranjitkan@chromium.org
, Mar 27 2017