MethodGrab / firefox-custom-new-tab-page

A Firefox extension that allows you to specify a custom new-tab URL
https://addons.mozilla.org/firefox/addon/custom-new-tab-page
ISC License
76 stars 18 forks source link

Add option to change tab title #2

Closed desto-git closed 6 years ago

desto-git commented 6 years ago

Since the webpage is inside an iframe, the title of my custom page is not shown in the tab. Because of type="content" it's also not possible to change it via JavaScript.

So I added an option to define a static title from the options page.

I personally don't want to have a title at all, but using an empty string will have no effect. So I use   instead. That's why I assign it to innerHTML.

Alternatively the title could be set via document.title. That won't allow HTML entities, but it would still be possible to insert the resulting character instead. I just find   to be clearer than an visually ambiguous space.

desto-git commented 6 years ago

Thanks for your response!

I rebased my master on yours, and in the process omitted the instructions commit. Now my commits are on top with the old timestamp still intact. I would need to force push onto my fork to make it public. Is my assumption correct? Would this pull request update or do I need to make a new one in this case?

Furthermore I noticed that lint-ext will log a warning for the innerHTML assignment. To keep it clean, I guess I should go with document.title after all. I tried to use the escapeHTML method described here, but it seems to be a Mozilla internal function. It will stop the warning, but will break the extension.

MethodGrab commented 6 years ago

I would need to force push onto my fork to make it public. Is my assumption correct? Would this pull request update or do I need to make a new one in this case?

Yes, force pushing your fork should update this PR.

Furthermore I noticed that lint-ext will log a warning for the innerHTML assignment. To keep it clean, I guess I should go with document.title after all.

If it's only a warning rather than an error it should be OK (as you said, there is no risk of XSS injection as it shows tags as plaintext) but using document.title it probably better.
As you can't set document.title to a space character, for your use case of having an empty title, you can use the unicode character 'invisible separator' instead (document.title = '\u2063')

desto-git commented 6 years ago

I went with document.title since my intention of using special characters works with both, and a stable build should be strived for.

Downside is that I have to use the resulting character. Using the unicode escape string won't work as the text input will automatically escape the \, resulting in the input being displayed as is. And trying to unescape it manually could have unintentional side effects.

Nonetheless, thank you for showing me the invisible separator. Unlike  , this also works in Chrome. Nice!

MethodGrab commented 6 years ago

Merged, thanks again for your contribution! I've submitted it to AMO for review; it should be available as v0.3.0 shortly.