w3c / css-validator

W3C CSS Validation Service
https://jigsaw.w3.org/css-validator/
Other
208 stars 105 forks source link

CssSelectors contexts are not computed in CssParseException #392

Closed AdrianSorop closed 1 year ago

AdrianSorop commented 1 year ago

Here is a quick JUnit test to reproduce the issue:

public void testPresentContextInErrors() throws Exception {

    String css = 
        "    td\n" + 
        "    {\n" + 
        "        color:something;\n" + 
        "    }";
    URL url = new URL("file://fake.css");
    Reader reader = new StringReader(css);
    ApplContext applContext = new ApplContext("en");
    applContext.setCssVersion("css3");
    applContext.setProfile("css3");
    applContext.setMedium("all");
    applContext.setWarningLevel(2);
    StyleSheetParser sheetParser = new StyleSheetParser(applContext);

    sheetParser.parseStyleElement(applContext, reader, null, applContext.getMedium(), url, 0);
    sheetParser.getStyleSheet().findConflicts(applContext);

    Errors errors = sheetParser.getStyleSheet().getErrors();
    assertEquals("“something” is not a “color” value", errors.getErrorAt(0).getException().getMessage());
    CssError error = errors.getErrorAt(0);
    CssParseException exception = (CssParseException) error.getException();

    StringBuilder messageBuilder = new StringBuilder();
    ArrayList<CssSelectors> contexts = exception.getContexts();
    if (contexts != null && !contexts.isEmpty()) {
      StringBuffer buf = new StringBuffer();
      for (Iterator<CssSelectors> iterator = contexts.iterator(); iterator.hasNext();) {
        CssSelectors selector = iterator.next();
        buf.append(selector);
        if (iterator.hasNext()) {
          buf.append(", ");
        }
      }
      if (buf.length() != 0) {
        messageBuilder.append("Context: ").append(buf).append(' ');
      }
    }
    if (exception.getProperty() != null) {
      messageBuilder.append("in property: ").append(exception.getProperty());
    }

    assertEquals("Context: td in property: color", messageBuilder.toString());
  }

I managed to fix this in the org.w3c.css.parser.analyzer.CssParser.ruleSet() method by moving the assigment of _currentContext = contextset; after the context_set was computed: _context_set = selectorlist() Here the code that worked for me:

/**
 * @exception ParseException exception during the parse
 */
void ruleSet() :
{ 
    ArrayList<CssSelectors> context_set = null;
    ArrayList<CssProperty> value_set = null;
    /////////////// PATCH START //////////////
    //  Commented, has no effect here; moved after the context_set is computed.
    //  currentContext = context_set;
    /////////////// PATCH END //////////////
}
{
    try {
    context_set = selector_list()

    /////////////// PATCH START //////////////
    {
        currentContext = context_set;
    }
    ///////////////PATCH END //////////////
......
ylafon commented 1 year ago

Thanks for the report, I modified slightly the proposed fix to make it only happen if the selector list is not empty, but good catch, it will make reporting far more accurate and useful.