Write title as innerText instead of innerHTML

[emilhem]

Description

Changing the title update to use innerText instead of innerHTML calms some security checks such as GitHub's own code checking.

tsc is grumpy now though with the error Property 'innerText' does not exist on type 'Element'.

Not sure if i agree with tsc fully.

Checklist

• I have read the contribution guidelines
• I have targeted this PR against the correct branch (master for website changes, dev for
  source changes)
• This is either a bugfix, a documentation update, or a new feature that has been explicitly
  approved via an issue
• I ran the test suite locally (npm run test) and verified that it succeeded

[geoffrey-eisenbarth]

Are you able to run the test suite? Let me know if you need any help!

[geoffrey-eisenbarth]

lgtm!

fyi I can't approve the PR, just trying to help other contributors :)

Edit: as @Telroshan pointed out, this should still be .innerText, the type cast just makes it treat titleElt as an HTMLElement instead of the Element | null type that is output by find().

I need more sleep.

[Telroshan]

I must admit I'm a bit confused here ; innerHTML is a property of the Element interface (see MDN docs), so we shouldn't even need a type cast to HTMLElement, unless I'm missing something?

Moreover, the PR mentions replacing innerHTML by innerText but it looks like the current PR simply adds a type cast comment without changing the accessed property, is that intended?

Could you also share the ouput of the failing security checks you mentioned in the PR description?

[geoffrey-eisenbarth]

@Telroshan The first commit changed .innerHTML to .innerText, but looks like when they added the type cast they reverted it back to .innerHTML.

You're right that .innerHTML is a property of Element, but as far as I can tell .innerText is not, which is why I suggested the type cast, since <title> will always be an HTMLElement.

Agreed it would be nice to have the output of the failing security test.

[emilhem]

Here's the output from GitHub's CodeQL

Unsure how I messed up the commit, but I'll commit now.

[emilhem]

Okay, so I accidentally rebased master then dev. That's why there was a bunch of extra stuff going on above.

The fix is in now, but the tests are failing since the type casting didn't seem to do the trick. Either I get

> [email protected] types-check
> tsc src/htmx.js --noEmit --checkJs --target es6 --lib dom,dom.iterable

src/htmx.js:4690:18 - error TS2339: Property 'innerText' does not exist on type 'Element'.

4690         titleElt.innerText = title
                      ~~~~~~~~~

Or if I move the type cast to where titleElt is set I get

> [email protected] types-check
> tsc src/htmx.js --noEmit --checkJs --target es6 --lib dom,dom.iterable

src/htmx.js:4688:13 - error TS2740: Type 'Element' is missing the following properties from type 'HTMLElement': accessKey, accessKeyLabel, autocapitalize, dir, and 123 more.

4688       const titleElt = find('title')
                 ~~~~~~~~

[emilhem]

Well, I borked the commit history obviously... I'll see what I can do to fix it..

[Telroshan]

Ok I see, thanks for the screenshot

Tbh, this kind of warning is kinda a lost cause regarding htmx as we support eval which is reported as dangerous by many scanners
Though, as the title tag can only contain text as per the docs, I see no reason to oppose to use ìnnerText here

As for the property itself, I would maybe recommend textContent instead of innerText, as

• It's a property of Node, which shouldn't require a cast at all since Element inherits it
• It doesn't trigger a reflow as opposed to innerText which must know what text is actually rendered

[Telroshan]

As for the branch, you would indeed want to rebase to dev and drop all the unnecessary commits

[emilhem]

Done! Thanks @Telroshan ! :)

[lukewarlow]

One benefit to this change is it's one less usage of a Trusted Types sink that will need handling if HTMX ever wants to support running in Trusted Types enforced environments.