Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with first-run html #784

Closed
dhowe opened this issue Jan 22, 2017 · 5 comments
Closed

Problem with first-run html #784

dhowe opened this issue Jan 22, 2017 · 5 comments

Comments

@dhowe
Copy link
Owner

dhowe commented Jan 22, 2017

screen shot 2017-01-22 at 1 21 35 pm

@dhowe dhowe changed the title Problem with first-run html Problem with first-run html (Firefox) Jan 22, 2017
@dhowe dhowe added this to the Release 3.2 milestone Jan 22, 2017
@cqx931 cqx931 changed the title Problem with first-run html (Firefox) Problem with first-run html Jan 22, 2017
@cqx931
Copy link
Collaborator

cqx931 commented Jan 22, 2017

https://github.com/dhowe/AdNauseam/blob/merged-1.9.16/src/js/i18n.js#L34
According to the latest ublock update. Anything else than <a>, <b>, <code>, <em>, <i>, <input>, and <span> will be rendered as plain text. Even class and id name.
If I change the locale to <span>filter lists<\/span>, instead of <span class='link' id='To3pfilter'>filter lists<\/span>, the rendering is fine.
If we don't want to change i18n.js, we nned to delete the class and id name from locale, and add them later in firstrun.js

@cqx931
Copy link
Collaborator

cqx931 commented Jan 22, 2017

firstrun page calls vAPI.i18n.render() to rerender the text for the changing firstrun-button.
But ublock is now using append for rendering i18n, so vAPI.i18n.render() could not be called multiple times and all the text need to be loaded at once.
The changing firstrun-button need to be switched to a css hide/display approach.

@dhowe
Copy link
Owner Author

dhowe commented Jan 22, 2017

alternatively we can change the i18n.js function, though we need to make sure any tags are safe (this is the underlying issue: gorhill#2084)

@cqx931
Copy link
Collaborator

cqx931 commented Jan 22, 2017

For class/id issue, I think it is better to just remove them from locale, as same effect can be achieved by writing a different css selector.

And the second issue is closely related to the core of this security issue: append instead of innerHTML. So I think it is better to leave the i18n.js unchanged and change the other.

@cqx931
Copy link
Collaborator

cqx931 commented Jan 22, 2017

fixed #786

@cqx931 cqx931 closed this as completed Jan 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants