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

Building this into a Chrome Extension - can't point to your script directly yet #87

Open
siakaramalegos opened this issue Aug 3, 2023 · 15 comments

Comments

@siakaramalegos
Copy link

siakaramalegos commented Aug 3, 2023

Hi there - thanks for making such a great tool. I love it, but sadly it does not work on pages that block iframes. So I'm working on building it into a Chrome Extension to try to get around this. Long term, it might be nice to have it there anyway for folks confused by bookmarklets.

I have not figured out the x-frame-options header issue yet, but assuming that eventually happens, another issue is I can't point to your script directly yet. This is because you're querying the document for a script tag to get the base URL. For now, I've pasted it directly there: https://github.com/siakaramalegos/resp-image-lint-extension/blob/main/RespImageLint.js#L1007C191-L1007C245

The idea is to use the same base code and point to your stuff for sponsoring, etc. This is just a convenience wrapper.

@siakaramalegos
Copy link
Author

I guess the question is, first, how do you feel about this? And secondly, if you like the idea, do you have a way you'd prefer to handle the baseURL issue that is easy for both of the bookmarklet and chrome extension to maintain?

@peter-neumann-dev
Copy link

Hi @siakaramalegos, I have already made a Chrome extension for myself as a workaround because I use a browser (Arc) that does not support bookmarklets yet. It is available in the chrome web store. You can check it out here: https://github.com/peter-neumann-dev/responsive-image-linter

For now, I kept the script just forked and changed the baseURL stuff as well. If the extension retrieves more updates frequently again, there should be a better way to keep it sync.

More here as well: #84

@ausi
Copy link
Owner

ausi commented Aug 3, 2023

how do you feel about this?

It’s great to see people using it. And I’m a little embarrassed that it works so unstable due to it being a bookmarklet. I still plan to transform it into a dev tools extension, but that might take a little longer still 😬
Having browser extensions from @siakaramalegos and @peter-neumann-dev to fill that gap in the meantime is totally fine with me. We should probably also link to them in the “Usage” section of the readme, wdyt?

… I can't point to your script directly yet. This is because you're querying the document for a script tag to get the base URL.

Would it help if I implement a fallback like this?

const scriptBase = script ? script.src.split('?')[0].replace(/[^/]+$/, '') : 'https://ausi.github.io/respimagelint/';

@siakaramalegos
Copy link
Author

@peter-neumann-dev very cool! I didn't even think of searching for an existing extension. Maybe we could combine our efforts. Might wait until I can figure out the iframe issue with my simple version first.

@ausi that's great! I think something like that would work great. I've done so much this week my brain didn't even think of using the code version of solving that problem lol. For referencing in the readme, that sounds great. Peter's is public and shareworthy. I need to figure out the iframe issue, and if I can get it to work, maybe I'll do a PR to Peter's repo. If not, I have another idea involving Cloudflare edge workers.

The back story is I'm a perf engineer so always have used the bookmarklet, mostly for the sizes attribute help. However, now I work at Shopify and it doesn't work on any of our sites since we send headers to block iframes. 😭

@peter-neumann-dev
Copy link

Fine for me as well 👍 Luckily, it was working on all my projects because there are no iFrames involved. 😃

The extension has around 300 users right now. I will do my best to keep it updated when there are changes in this repo.

@siakaramalegos
Copy link
Author

I got it working!! https://github.com/siakaramalegos/resp-image-lint-extension

@peter-neumann-dev do you prefer to merge in my code or maintain a separate extension? Yours has a more complex file structure, so I'm guessing you'd want to structure it differently than I did. Let me know what you're thinking either way!

@ausi if you can make that tiny script change, it will be easier for us to maintain extensions too.

@siakaramalegos
Copy link
Author

Unrelated - noticed this warning in the extension logs in case you wanted to optimize the code slightly @ausi

Canvas2D: Multiple readback operations using getImageData are faster with the willReadFrequently attribute set to true. See: https://html.spec.whatwg.org/multipage/canvas.html#concept-canvas-will-read-frequently

@peter-neumann-dev
Copy link

@siakaramalegos Nice! 🎉 I think the structure in my extension is looking more complex than it looks at a first glance. As I can see from your commit, it seems we only have to adjust two files:

If you like, you can create a pull request, or I'll add the same changes in mine extension. Do you have an example page where I can test the previous not working linting?

@siakaramalegos
Copy link
Author

Yes, any Shopify store, but for convience here's our blog that we also built on shopify: https://performance.shopify.com/

@peter-neumann-dev
Copy link

Yes, I can reproduce it. Will you create a PR, or should I add the changes myself @siakaramalegos?

@ausi
Copy link
Owner

ausi commented Aug 4, 2023

if you can make that tiny script change

Done in f2002aa

noticed this warning in the extension logs in case you wanted to optimize the code slightly

Done in 134a5ae

@ausi
Copy link
Owner

ausi commented Aug 4, 2023

We should probably also link to them in the “Usage” section of the readme, wdyt?

I added a link to Peters extension now: https://github.com/ausi/respimagelint#usage

@siakaramalegos I’d also add a link to your extension once finished. Can you notify me then?

@siakaramalegos
Copy link
Author

siakaramalegos commented Aug 4, 2023 via email

@peter-neumann-dev
Copy link

I have released a new version of the extension and submitted the update to the Chrome Web Store for review. Once it is approved, the new version will be available automatically through the store. 🤞🏻😃

@peter-neumann-dev
Copy link

It is now published finally. After updating, you have to reactivate and accept to the new permission that it can block content. Afterward, it is working everywhere in my tests 👍🏻 🙂

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