Menu

#504 HTML mode: Fix mode properties in <script> blocks.

None
closed-accepted
None
5
2014-02-15
2013-07-13
No

Fix #3614631 by delegating to script modes, instead of just importing their rules.

The indentation rules for the script mode are only applied if (real) delegation is used. (Some relevant code can be found in JEditBuffer.getIndentRules)

This patch also shortens and simplifies the file nicely, but for the price of reintroducing a corner-case bug: Closing </script> tags may be consumed by script comments, which is inconsistent with the XML plugin and most browsers.

Given that it seems impossible to fix this (in a mode file) while using (real) delegation, I'd say that having correct script mode properties is preferable to not having this bug.

(Thanks to Matthieu Casanova for pointing in the right direction.)

Discussion

  • Marc Häfner

    Marc Häfner - 2013-07-13

    HTML mode: Fix bug 3614631 - mode properties in script blocks

     
  • Marc Häfner

    Marc Häfner - 2013-11-08
    • assigned_to: nobody --> daleanson
     
  • Marc Häfner

    Marc Häfner - 2013-11-08

    Added new version, that also includes a fix for #3615189. (Also renames STRING ruleset to ATTRIBUTE and removes an outdated comment.)

     
  • Marc Häfner

    Marc Häfner - 2013-11-08

    Fixes 3614631, 3615189, 3615174

     
  • Dale Anson

    Dale Anson - 2013-11-11

    Marc, could you post an example of "Closing </script> tags may be consumed by script comments, which is inconsistent with the XML plugin and most browsers." ?

    This patch is looking good to me, and as far as I can tell, even putting a javascript comment in front of the closing script tag looks good.

     
  • Marc Häfner

    Marc Häfner - 2013-11-12

    Any plain old

    <script> // comment </script>

    consumes the closing tag (because the comment is matched by an EOL_SPAN). Subsequent text is still highlighted with JavaScript mode, but a look at the SideKick or a test with a browser reveals that it's actually HTML. (Fixing this was one of the goals of #3595104, but it seems to have caused more trouble than good.)

     
  • Eric Le Lay

    Eric Le Lay - 2013-11-26

    Hi,
    looks good to me.

    What about marking the whole end of attribute invalid when invalid entity is detected ?
    replace <SEQ>...</SEQ> with <SEQ_REGEXP TYPE="INVALID">&amp;[^"]*</SEQ_REGEXP>
    Otherwise it's rather easy to miss those red &amp; in pink url.

    Also, the old html mode will show all the javascript code as comment when viewing the source of this page:
    http://www.cs.umd.edu/~bederson/papers/index.html
    which the new mode doesn't, which is good!

     
  • Marc Häfner

    Marc Häfner - 2013-11-27

    > ... it's rather easy to miss those red &amp; in pink url.

    Yes that is an issue, but I think that would be better addressed by changing the default colors.

    My motivation are users who are forced to work with broken, but nonetheless perfectly well displayed, HTML files. And I strongly suspect HTML5 makes such a scenario even more likely (it "legalizes" some of those broken cases, to be exact).

    An overly aggressive error display wouldn't be helpful to those users but a serious distraction. Even more so if they don't use the default color schema (for example my INVALIDs are _really_ red.)

     
  • funa take

    funa take - 2013-12-08

    When jsp files are loaded, following error is displayed.
    Invalid delegate: html::JAVASCRIPT
    Invalid delegate: html::CSS

     
  • Dale Anson

    Dale Anson - 2013-12-12

    Marc, would you take a look my version 3, attached? I believe it fixes both the
    <script>// comment </script>
    issue and the invalid delegate issue that funatake reported.

     
  • funa take

    funa take - 2013-12-13

    It looks good to me.
    Thank you.

     
  • Marc Häfner

    Marc Häfner - 2013-12-16

    Version 3 doesn't really solve the problem of script-comments consuming html-tags. If I understand TokenMarker etc. correctly (and testing seems to confirm that) one can either:

    • use <IMPORT DELEGATE=... /> to be able to "override" the offending EOL_SPANs. This imports the rules into the current rule set. But it does not import any mode properties (the original issue for this ticket)

    • use real delegation (i.e. DELEGATE on SEQ or SPAN), where we get the (original) RuleSet with the appropriate modeName and thus the correct mode properties. But it seems impossible to somehow inject rules into another mode / rule set.

    But version 3 fixes the delegate issue nicely. (Sorry for the oversight and thanks for testing.) I attached a version 4 which includes only those changes from V3.

     

    Last edit: Marc Häfner 2013-12-27
  • Dale Anson

    Dale Anson - 2014-02-15

    Applied version 4 in svn revision 23418, this patch fixes several other bugs in the html mode. Thanks, Marc!

     
  • Dale Anson

    Dale Anson - 2014-02-15
    • status: open --> closed-accepted
    • Group: -->
     

Log in to post a comment.