|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
Created attachment 8784607 [details] testcase 1 STR: 1. Load attached testcase. 2. Inspect any of the three lines of text, in the CSS "Rules" view. 3. Compare to the "computed style" view to see what actually took effect. ACTUAL RESULTS: - The "rules" view shows the original author-provided styles, with the comments removed, which converts e.g. 1/*ThisIsAComment*/2; into "12", and "5px/**/0/**/0/**/20px/**/;" into "5px0020px". - It also strikes out the last one ("5px0020px") as being invalid CSS, despite the fact that the actual declaration (with CSS comments) *does* parse successfully and does get honored, as you can see in the rendering & the computed-style view. EXPECTED RESULTS: - The "rules" view should preserve CSS comments inside of a CSS property's value. Or, it should collapse them away to "/**/", if needed. Or replace them with a blank space -- that would be fine with these properties, at least, though I'm not sure if it's fine in general. For comparison, Chrome preserves the comments in their entirety, in its equivalent of our DevTools "CSS Rules" view.
Created attachment 8784614 [details] screenshot #1 For illustration, here's a screenshot showing the 3rd part of the testcase -- in particular, it shows: - The source code (upper right) - DevTools' comments-stripped-out version of the "margin" declaration (lower right), struck out to indicate that it's invalid CSS. - The actual rendering (upper left), with the margin clearly being honored on "Me too", despite the strikethrough in the rules view.
9 months ago
9 months ago
P1 because the code is valid and we show a bogus output. Tom: do you mind giving pointers about how this can be solved?
There are two separate bugs here. One is the token pasting. I'll attach a patch for this momentarily. The other bug is that the comments aren't preserved in the rule view. This is fixable but on irc :pbro said he'd rather we not.
BTW if you want to try comment-preserving, the spot touched by this patch needs to change, but output-parser also needs to change to not drop comments.
Thanks a lot Tom for your prompt analysis and patch. I think this is a rather urgent bug because it causes the rule-view to show incorrect data (which is why I had triaged it as a P1), so I lean towards fixing it the way you did first, and then revisiting this decision later if we think there's a user need to show comments in the rule-view.
Comment on attachment 8785963 [details] Bug 1297890 - don't allow token pasting in parseDeclarations; https://reviewboard.mozilla.org/r/74972/#review73128
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/18f8a36a4ed2 don't allow token pasting in parseDeclarations; r=pbro
I have to reproduced this bug with Nightly 51.0a1 (2016-08-24) on Windows 7 64 bit; The Bug's fix is verified on beta Build ID 20170105155013 User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0 [Testday-20170106]
I have to reproduced this bug with Nightly 51.0a1 (2016-08-24) on Ubuntu 16.10, 64 bit! The Bug's fix is verified on beta. Build ID 20170105155013 User Agent Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0 [Testday-20170106]
Thanks guys for verifying this bug.