kiwix-js
kiwix-js copied to clipboard
kiwix
Remove usage of innerHTML of our code
It is currently the case in app.js and uiUtil.js, but it should be searched everywhere (excluding the third-party libraries like fontawesome or jquery).
.innerHTML can be unsafe in terms of security. Its usage triggers warnings (at least when we upload the package on Mozilla store).
It should be possible to replace them by .textContent when it's unformatted text.
When it's not simple text, it should be possible to create the nodes programmatically.
Where it's not too complicated, I agree. The safety issues are minimal given that we only inject html from ZIM archives. But I guess it's possible a ZIM could unintentionally have malicious code.
It's true that the usage we do of .innerHTML is probably not risky.
But the automatic tools can not know that. Plus using .textContent is probably a bit faster (as the browser does not have to convert HTML to/from the DOM)
In any case, this is not high priority.
@mossroy Hey, if it is a valid and possible issue then I would like to work on this. please let me know. Thanks:)
Thanks @prsndp I've assigned it to you. Please read the CONTRIBUTING.md guidelines before creating a PR
Hey, @mossroy if this issue is still available, I would like to work upon it. Please do let me know if this is the case.
@me-aashish Thanks for the offer. Please see https://github.com/kiwix/kiwix-js/blob/master/CONTRIBUTING.md for information on how to get started. When you've set up and have looked around the code a little, come back here and let us know how you plan to tackle the issue.
Sure. Thanks!
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Jaifroid @.> Sent: Sunday, June 12, 2022 2:45:10 PM To: kiwix/kiwix-js @.> Cc: Aashish Kumar Jha @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Remove usage of innerHTML of our code (Issue #824)
@me-aashishhttps://github.com/me-aashish Thanks for the offer. Please see https://github.com/kiwix/kiwix-js/blob/master/CONTRIBUTING.md for information on how to get started. When you've set up and have looked around the code a little, come back here and let us know how you plan to tackle the issue.
— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/824#issuecomment-1153110120, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AS274UT7XLRNBQS65GRY5DLVOWTB5ANCNFSM5NVQKE2A. You are receiving this because you were mentioned.Message ID: @.***>
By looking at the codebase, I came to know that besides app.js and uiUtil.js, .innerHTML is used several times in node_modules file. So I have to replace .innerHTML from node_modules file too ?
@me-aashish No, these are NPM libraries that are only used during development. It's only in our own app's files that we need to find alternatives for .innerHTML.
I am having problem in setting up local web server. Can you please guide me once ?
Make sure you've set up node and npm on your system. Then run npm install --global http-server.
I have created a PR changing the contents of only app.js. Will also modify uiUtil.js once you verify if all the things are working well. So, can you please review my PR? #873
You have to test your changes first, by running the app on your branch (see https://github.com/kiwix/kiwix-js/blob/master/CONTRIBUTING.md ). Did you already did that? Which tests did you do?
Firstly, ServiceWorker API was working on my PC but not it is not working right now while testing on my local web http-server while it is working fine if I try to run the website without the local server. Can you tell possible reasons for that.
Check that you run the webserver from the root of the git repo (not from a subdir). That might be the reason. ServiceWorker mode should always work on http://localhost URLs
Ok! Got you!
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Mossroy @.> Sent: Monday, June 13, 2022 10:36:28 AM To: kiwix/kiwix-js @.> Cc: Aashish Kumar Jha @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Remove usage of innerHTML of our code (Issue #824)
Check that you run the webserver from the root of the git repo (not from a subdir). That might be the reason. ServiceWorker mode should always work on http://localhost URLs
— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/824#issuecomment-1153476566, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AS274UQQH7WAERR2DXKULX3VO26VJANCNFSM5NVQKE2A. You are receiving this because you were mentioned.Message ID: @.***>
Now, the server is working fine on http://localhost:8080/www/index.html but the changes I make in the code is not getting reflected back in http://localhost URL. Please can you tell some possibilities for that ?
Please re-read completely the CONTRIBUTING.md file, where you should find your answer, and other important directions.
And just a quick tip: once you select the "Bypass App Cache" setting (only available when you have turned on Service Worker mode), you can refresh everything by having Dev Tools open and pressing Ctrl-Shift-R (not just Ctrl-R) for a complete re-load of the app. Just do this after each significant code change.
Thanks a lot!
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Jaifroid @.> Sent: Monday, June 13, 2022 1:15:24 PM To: kiwix/kiwix-js @.> Cc: Aashish Kumar Jha @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Remove usage of innerHTML of our code (Issue #824)
And just a quick tip: once you select the "Bypass App Cache" setting (only available when you have turned on Service Worker mode), you can refresh everything by having Dev Tools open and pressing Ctrl-Shift-R (not just Ctrl-R) for a complete re-load of the app. Just do this after each significant code change.
— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/824#issuecomment-1153585889, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AS274US7HUSA3U4NJKJRSZLVO3RJJANCNFSM5NVQKE2A. You are receiving this because you were mentioned.Message ID: @.***>
When I replace .innerHTML with .textContent, changes are visible only in ServiceWorker mode not in JQuery mode in localhost url. Why it is happening?
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Aashish Kumar Jha @.> Sent: Monday, June 13, 2022 1:43:49 PM To: kiwix/kiwix-js @.> Subject: Re: [kiwix/kiwix-js] Remove usage of innerHTML of our code (Issue #824)
Thanks a lot!
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Jaifroid @.> Sent: Monday, June 13, 2022 1:15:24 PM To: kiwix/kiwix-js @.> Cc: Aashish Kumar Jha @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Remove usage of innerHTML of our code (Issue #824)
And just a quick tip: once you select the "Bypass App Cache" setting (only available when you have turned on Service Worker mode), you can refresh everything by having Dev Tools open and pressing Ctrl-Shift-R (not just Ctrl-R) for a complete re-load of the app. Just do this after each significant code change.
— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/824#issuecomment-1153585889, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AS274US7HUSA3U4NJKJRSZLVO3RJJANCNFSM5NVQKE2A. You are receiving this because you were mentioned.Message ID: @.***>
@me-aashish The code for jQuery mode and the code for Service Worker mode are quite separate in lots of places. I.e., if you're in one mode, a lot of the code in the other mode is not running, and vice-versa. Code in the backend is common to both modes.
So for that I have to make changes separately in the jQuery and Service Worker mode ?
Get Outlook for Androidhttps://aka.ms/AAb9ysg
From: Jaifroid @.> Sent: Monday, June 13, 2022 3:27:27 PM To: kiwix/kiwix-js @.> Cc: Aashish Kumar Jha @.>; Mention @.> Subject: Re: [kiwix/kiwix-js] Remove usage of innerHTML of our code (Issue #824)
@me-aashishhttps://github.com/me-aashish The code for jQuery mode and the code for Service Worker mode are quite separate in lots of places. I.e., if you're in one mode, a lot of the code in the other mode is not running, and vice-versa. Code in the backend is common to both modes.
— Reply to this email directly, view it on GitHubhttps://github.com/kiwix/kiwix-js/issues/824#issuecomment-1153717352, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AS274UXGJCFGUAC34PBGUW3VO4AYPANCNFSM5NVQKE2A. You are receiving this because you were mentioned.Message ID: @.***>
I have performed all the tests which includes testing in both jQuery and Service Worker mode. Also tested the extension version as stated in CONTRIBUTING.md and these two PRs are made after all the tests. Kindly review them. #874 #873
Hello, I have some experience with reactjs and javascript, so I want to work on this issue. Please guide me how to set up this project initially, and then assign this to me
