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

Use Zepto.js instead of jQuery where possible #670

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use Zepto.js instead of jQuery where possible #670

wants to merge 1 commit into from

Conversation

mehrdadn
Copy link

@mehrdadn mehrdadn commented Jan 13, 2018

Hi! This is an update to address the chromeIPass performance issue due to jQuery; it is intended to resolve issue #550.

It is somewhat ad-hoc and not well-written, but I think it should (?) work correctly. The idea is to use Zepto.js as a lightweight alternative to jQuery where possible, and load jQuery dynamically when input fields are actually detected. This prevents most subframes from loading jQuery, since generally few subframes have input fields.

I mainly tried to minimize the diff and avoid impacting the old code where possible (to make it easier to compare/understand), so that's why the code isn't very polished. I can fix the indentation issues, etc. if desired; it was a bit difficult to do this non-invasively since I would have had to re-indent and re-touch existing code as well.

One thing to note is that I also replaced the setInterval call with MutationObserver. I'm not sure whether the observe() call needs attributes: true instead of attribute: false. Please change this if needed. (I didn't set it lest there is a performance impact.)

It would be great if someone could also test this and ensure that:

  1. The performance is actually better.
  2. Nothing previously-working is broken (including when pages change dynamically).

Testing it is important as I've only done limited testing with it on one machine.

@mehrdadn
Copy link
Author

Bump?

@pfn
Copy link
Owner

pfn commented Feb 1, 2018 via email

@mehrdadn
Copy link
Author

Actually... I think I've found bugs in this. :\ I'll try to update it when I get the chance (but it might be a while from now).

@ForsakenHarmony
Copy link

You can probably do it without either

@mehrdadn
Copy link
Author

mehrdadn commented Mar 31, 2018

Update: I fixed the bug I had found (subframes were throwing errors).
However, it's still not ideal. The problem is that chromeIPass makes the fundamentally incorrect assumption that all input fields become available together, and that they do so at a "nice" time. This isn't always the case (Javascript can add/remove fields to a form), and and when this assumption fails to hold, the password-generation "key" icon no longer auto-appears.
Addressing this issue would seem to require quite a bit of surgery (essentially rewriting much of the extension), so I'm not sure I'll ever get the chance to address it.

Regarding jQuery vs. Zepto.js, Zepto.js might be avoidable with some care, but jQuery-UI is definitely required for the password-generation dialog UI; there's no getting around that, unless you write your own equivalent UI code.

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

Successfully merging this pull request may close these issues.

3 participants