#560 XML Input tags won't fold

Bug
closed-fixed
Neil Hodgson
Scintilla (789)
3
2007-06-22
2007-04-30
Chad Burrus
No

In an XML document, if you have an "input" tag that is not self-closing (the tag looks like "<input>...</input>" instead of XHTML default "<input/>"), you can't fold it. It's more of an annoyance than a real problem, but I've attached a screenshot to show this problem. In the screenshot, I'm editing an XSLT document in SciTE 1.73 (official build as of March 31, 2007) on Windows XP.

I looked through the source of the current HTML lexer, which is used to parse XML also, and found the problem, though I don't know enough about how the lexers in SciTE work to fix it. It's in the "classifyTagHTML" function, lines 239 - 247. From what I think is going on, the current tag is always compared against a list of self-closing tags, regardless if we're lexing HTML or XML. If the current tag is in the list, we don't fold it. Removing these lines causes input tags to fold in both XML and HTML.

The current behavior is correct for HTML, but it's slightly off for XML. (There is a comment that says the current check is only a quick hack, so this may be a known bug, but it wasn't in the bug list here. My apologies if this is a repeat.) From what I can see, fixing this would be as simple as figuring out if we're lexing XML or HTML, and then skipping the don't fold behavior for XML documents. However, from my limited tests, figuring out the current language is somewhat difficult. I'll keep looking into this myself, but I'm hoping it will be a rather simple fix for someone who understands how things work.

Discussion

  • Chad Burrus
    Chad Burrus
    2007-04-30

    Screenshot showing problem

     
    Attachments
  • Chad Burrus
    Chad Burrus
    2007-04-30

    • priority: 5 --> 2
     
  • Neil Hodgson
    Neil Hodgson
    2007-05-02

    Logged In: YES
    user_id=12579
    Originator: NO

    Looks like a valid bug. I won't be working on this.

     
  • Neil Hodgson
    Neil Hodgson
    2007-05-02

    • priority: 2 --> 3
    • assigned_to: nobody --> nyamatongwe
    • status: open --> open-accepted
     
  • Chad Burrus
    Chad Burrus
    2007-05-04

    Logged In: YES
    user_id=1286799
    Originator: YES

    I found a workable solution (at least from my perspective) and I've attached a patch showing my changes. In the patch, LexHTML.cxx is the checked-in version at release 1.73 and LexHTML.cxx.mod contains my modifications.

    Basically, the fix involves setting a global variable to indicate whether or not the current language is XML. To do this, I changed the XML LexerModule definition at the end of the file to call a new function that sets the global variable to true and then calls the old lexing function. I also changed the HTML version to set that global variable to false and then call the old lexing function. I also wrapped those lines I mentioned earlier in an if test based on that global variable. On my machine, this fixes the problem for both HTML and XML.
    File Added: LexHTML.cxx.diff

     
  • Neil Hodgson
    Neil Hodgson
    2007-05-04

    Logged In: YES
    user_id=12579
    Originator: NO

    That won't work in the general case since lexers need to be reentrant. An application may have both an XML and HTML document open at the same time and setting styles or folding may result in a notification to the container which may then lead to some work using the other language's lexer resulting in a confusing mix of modes. It is even possible for multiple threads to call this lexing code which will also confuse the situation. Global variables should almost always be avoided. Have a look at passing this information as a parameter.

     
  • Chad Burrus
    Chad Burrus
    2007-05-05

    Logged In: YES
    user_id=1286799
    Originator: YES

    I was afraid you were going to say something along those lines. I was able to switch back and forth between an XML document and an HTML document in my local SciTE instance without a problem, but I can easily see where multiple threads or a more complicated situation would be an issue.

    I've adjusted my patch to pass a parameter into the real lexing function, as you suggested. For XML, the LexerModule calls an intermediary function that passes in the parameter to the real lexing function. The lexing function calls the classifyTagHTML eventually, and passes this parameter on, so the classifyTagHTML will allow folding for input tags and the like. For HTML, the same process occurs, though the parameter is false. I've replaced the previous patch file with my new version.

    I've also noticed a related problem with script tags. It's another situation that's correct for HTML but incorrect for XML. Basically, the first closing tag after a script tag closes that script tag, even if it's not a script tag that closes. In XML, however, that shouldn't be the case. Only a script tag should close a script tag in XML, and since I was working on it anyway, I fixed that error also. I'm going to keep that separate, though, since technically that's another bug. I'll upload a new file that contains the entire patch for both bugs, since it was only a one line fix.
    File Added: LexHTML.cxx.diff

     
  • Chad Burrus
    Chad Burrus
    2007-05-05

    Logged In: YES
    user_id=1286799
    Originator: YES

    File Added: LexHTML.cxx.mod2

     
  • Chad Burrus
    Chad Burrus
    2007-05-06

    Logged In: YES
    user_id=1286799
    Originator: YES

    I just noticed I uploaded the wrong file. Here's the correct one.
    File Added: LexHTML.cxx2.diff

     
  • Chad Burrus
    Chad Burrus
    2007-05-06

    Patch for LexHTML.cxx

     
    Attachments
  • Chad Burrus
    Chad Burrus
    2007-05-06

    Logged In: YES
    user_id=1286799
    Originator: YES

    I found a bug created by my previous patch, so I fixed the problem and am updating my patches.

    Basically, with the previous version, script in XML (and I assume HTML) would sometimes not be colored correctly. I'm not sure why it wasn't colored correctly, but pulling portions of the code I had surrounded by the if back outside ihe test seemed to fix the problem.
    File Added: LexHTML.cxx.diff

     
  • Chad Burrus
    Chad Burrus
    2007-05-06

    Logged In: YES
    user_id=1286799
    Originator: YES

    File Added: LexHTML.cxx2.diff

     
  • Chad Burrus
    Chad Burrus
    2007-05-06

    Patch for both bugs

     
    Attachments
  • Neil Hodgson
    Neil Hodgson
    2007-05-07

    Logged In: YES
    user_id=12579
    Originator: NO

    Committed with formatting and naming edits.

     
  • Neil Hodgson
    Neil Hodgson
    2007-05-07

    • status: open-accepted --> open-fixed
     
  • Neil Hodgson
    Neil Hodgson
    2007-06-22

    • status: open-fixed --> closed-fixed