ansi escapes in custom source markers

[romainfrancois]

Intent

addresses #12425

Approach

Avoid to create an AceAnnotation with both .text and .html because otherwise .text is prefered as per this code in ace:

var annoText = annotation.text;
annoText = annoText ? lang.escapeHTML(annoText) : annotation.html || "";

Alternatively, the ace code could use annotation.html ? instead of annotation.text ? so that the html version would have priority.

Automated Tests

Indicate whether this change includes unit tests or integration tests, or link a work item or pull request to add those tests to another repo. If the change cannot or will not be covered by a test, indicate why.

QA Notes

Save this file as test.R in the current working directory:

tmpfile <- "test.R"

foo <- cli::bg_br_white(cli::col_red("I am red!"))
bar <- cli::bg_br_yellow(cli::col_cyan("I am cyan!"))

markers <- list(
  list(
    type = "error",
    file = tmpfile,
    line = 6,
    column = 1,
    message = structure("<i>I am Groot</i>", class = "html")
  ),
  list(
    type = "info", 
    file = tmpfile,
    line = 12,
    column = 1,
    message = bar))

.rs.api.sourceMarkers("Test Name", markers)

and then run it.

Expected:

Markers tab:

Annotations:

Checklist

• If this PR adds a new feature, or fixes a bug in a previously released version, it includes an entry in NEWS.md
• If this PR adds or changes UI, the updated UI meets accessibility standards
• A reviewer is assigned to this PR (if unsure who to assign, check Area Owners list)
• This PR passes all local unit tests

[romainfrancois]

cc @timtmok or should the structure(class = "html") path be removed ? e.g. this would need I believe some tweaks around:

rstudio/src/cpp/session/modules/SessionMarkers.cpp

Line 402 in b735c15

"messageHTML", messageHTML);

to entirely avoid already structured html, and the risks of js injection it brings.

[romainfrancois]

Related to #12048, an orthogonal follow up could be to support annotations to carry an html widget. This would be safer as they are wrapped around an <iframe>

[romainfrancois]

TODO: this breaks the source markers:

Enregistrement.de.l.ecran.2022-12-14.a.13.42.26.mov

[timtmok]

Nice!

[romainfrancois]

Following up on #10278 I think we have to give up on the structure(, class = "html") because e.g. we could have message = structure('<i onclick = "alert(42)">I am Groot</i>', class = "html") which does alert() :

[romainfrancois]

Disabled it so that custom source markers can't emit already prepared html content:

tmpfile <- "test.R"

foo <- cli::bg_br_white(cli::col_red("I am red!"))
bar <- cli::bg_br_yellow(cli::col_cyan("I am cyan!"))

markers <- list(
  list(
    type = "error",
    file = tmpfile,
    line = 6,
    column = 1,
    message = structure('<i onclick = "alert(42)">I am Groot</i>', class = "html")
  ),
  list(
    type = "info", 
    file = tmpfile,
    line = 12,
    column = 1,
    message = bar))

.rs.api.sourceMarkers("Test Name", markers)

gives:

markers that we make internally may be html, but I don't think this is used at the moment.

We can consider a followup where we would allow creating markers/annotations that would consist of some html widget, that we could embed as an <iframe> so be less risky.