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

Remove inline JS from pad.html and timeslider.html #504

Closed
drdla opened this issue Feb 25, 2012 · 3 comments
Closed

Remove inline JS from pad.html and timeslider.html #504

drdla opened this issue Feb 25, 2012 · 3 comments

Comments

@drdla
Copy link
Contributor

drdla commented Feb 25, 2012

To improve maintainability and page loading speed, inline JS should be removed from HTML files.

Inline scripts block the rendering of the page.
Inline event handlers do not respect proper separation of concerns and make the code base harder to maintain.

@cweider
Copy link
Contributor

cweider commented Feb 26, 2012

Hm, I don’t understand. The pages timeslider.html and page.html seems fine for to me. Yes, there are 10 lines of script at the bottom, but that’s boot code that is unavoidable. The pad and time slider modules were created to do what I think you want to do. (See #371)

@drdla
Copy link
Contributor Author

drdla commented Feb 26, 2012

The editbar for example uses onClik event handlers directly written into the HTML, which would better be done calling something like $('#users').click(function() {...}); from a separate JS file. This improves maintainability, since it is clear, where code regarding functionality (JS) and code regarding structure (HTML) is kept.

An inline script causes the rendering of the page to be blocked until all JS code is downloaded and parsed. Until then, the user is presented no or a broken interface, which obviously is not desirable.
I haven´t investigated very much, how exactly all the modules interact etc., but as far as I can see, the paths do not depend on anything specific about the client and thus could be moved to an external JS file. The same appears to hold true for the require statements that could as well be called from an external file.

@cweider
Copy link
Contributor

cweider commented Feb 26, 2012

Ok, yea, all the onclick attributes need to go away (I have a patch for that). I also agree that the <script /> elements in the page are not desirable. I’m working on slowly removing them - the first priority is to move SocketIO into the pad.js package and then to load it asynchronously require('/pad.js', function (pad) {/*...*/}) (I just need to be very careful that this preserves debuggability). The good news is that, once cached, the difference is negligible and #449 improves the use of Expires and 304’s.

Eventually, I would want only require-kernel and the inline boot script on the page.

@cweider cweider mentioned this issue May 13, 2012
muxator added a commit that referenced this issue Dec 18, 2019
This fixes some security vulnerabilites, among them an arbitrary file overwrite.


The output of `npm audit` goes from this:
  found 17 vulnerabilities (15 low, 2 high) in 13344 scanned packages
    run `npm audit fix` to fix 6 of them.
    1 vulnerability requires semver-major dependency updates.
    10 vulnerabilities require manual review. See the full report for details.

To this:
  found 5 vulnerabilities (3 low, 2 high) in 13370 scanned packages
    1 vulnerability requires semver-major dependency updates.
    4 vulnerabilities require manual review. See the full report for details.


Changelog:
- https://github.com/npm/cli/releases


6.13.4 (2019-12-11)
    BUGFIXES
    320ac9aee npm/bin-links#12 npm/gentle-fs#7 Do not remove global bin/man links inappropriately (@isaacs)

    DEPENDENCIES
    52fd21061 gentle-fs@2.3.0 (@isaacs)
    d06f5c0b0 bin-links@1.1.6 (@isaacs)

6.13.3 (2019-12-09)
    DEPENDENCIES
    19ce061a2 bin-links@1.1.5 Properly normalize, sanitize, and verify bin entries in package.json.
    59c836aae npm-packlist@1.4.7
    fb4ecd7d2 pacote@9.5.11
        5f33040 #476 npm/pacote#22 npm/pacote#14 fix: Do not drop perms in git when not root (isaacs, @darcyclarke)
        6f229f7 sanitize and normalize package bin field (isaacs)
    1743cb339 read-package-json@2.1.1

6.13.2 (2019-12-03)
    BUG FIXES
    4429645b3 #546 fix docs target typo (@richardlau)
    867642942 #142 fix(packageRelativePath): fix 'where' for file deps (@larsgw)
    d480f2c17 #527 Revert "windows: Add preliminary WSL support for npm and npx" (@craigloewen-msft)
    e4b97962e #504 remove unnecessary package.json read when reading shrinkwrap (@Lighting-Jack)
    1c65d26ac #501 fix(fund): open url for string shorthand (@ruyadorno)
    ae7afe565 #263 Don't log error message if git tagging is disabled (@woppa684)
    4c1b16f6a #182 Warn the user that it is uninstalling npm-install (@Hoidberg)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants