Closed Bug 1211479 Opened 6 years ago Closed 6 years ago
Errors when parsing substituting variables should include the generated string to help diagnose the issue
See bug 1211221 for an example of this. Variables may have many different values, some of which may cause parsing issues while other may work fine. If a parsing issue is encountered, the error text may read: > Error in parsing value for 'margin-bottom' after substituting variables. Falling back to 'initial'. It would be an improvement to make this error text read: > Error in parsing value for 'margin-bottom' after substituting variables. Falling back to 'initial'. Generated value was 'calc(-1 * 0)'. This would help the developer find which variable value was the cause of the parsing issue quicker.
Looks like PEValueWithVariablesParsingError is the string here. Maybe `expandedValue` can be added to the error message at this point: https://dxr.mozilla.org/mozilla-central/source/layout/style/nsCSSParser.cpp#2693.
I came to the same conclusion! I'm testing a patch now :)
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Try push, https://treeherder.mozilla.org/#/jobs?repo=try&revision=2cd5f63a896b I had compiler issues on my local build with the ErrorReporter.cpp changes, but maybe it will be green on try (suggesting I needed a clobber build to fix some MSVC bug).
Comment on attachment 8669822 [details] [diff] [review] Patch You need to change the string name when you change its value in a semantically-incompatible (as in, requires new localization work) way, so the l10n tools will pick it up correctly.
Comment on attachment 8669822 [details] [diff] [review] Patch I'm not really familiar with ErrorReporter.h/cpp, so I'll defer to a style-system peer on this (heycam, bz, or dbaron). --> Canceling review. But yes, bz's comment applies -- you need a new string name here in css.properties.
Comment on attachment 8669863 [details] [diff] [review] Patch v1.1 > +PEValueWithVariablesParsingErrorInValue=Error in parsing value for '%1$S' after substituting variables. Generated value was '%2$S'. Probably good to document that %1$S will be the property name. r=me
Attachment #8669863 - Flags: review?(bzbarsky) → review+
You need to log in before you can comment on or make changes to this bug.