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

mode/bookmarklets: Fix darken / dark-mode #2971

Closed
wants to merge 1 commit into from

Conversation

shamazmazum
Copy link
Contributor

Hi! I've removed a preprocessing step which used quri to parse links like javascript:<javascript code>. It worked incorrectly and darken was broken because of this:

CL-USER> (quri:uri-path
           (quri:uri
             "javascript:document.querySelectorAll('*').forEach(
               e=>e.setAttribute('style','background-color:#222 !important;background-image:none !important;color:#' +
                                 (/^A|BU/.test(e.tagName) ? '36c;text-decoration:underline;' : 'eee;') +
                                 e.getAttribute('style')))"))
"document.querySelectorAll('*').forEach(
               e=>e.setAttribute('style','background-color:"

(I've added few newlines for this example, but it doesn't really matter.)

Also, javascript code is now executed as is. Preprocessing
with quri is removed. A reason for this:

NYXT-USER> (quri:uri-path (quri:uri "javascript:document.querySelectorAll('*').forEach(e=>e.setAttribute('style','background-color:atlas-engineer#222 !important;background-image:none !important;color:#'+(/^A|BU/.test(e.tagName)?'36c;text-decoration:underline;':'eee;')+e.getAttribute('style')))"))
"document.querySelectorAll('*').forEach(e=>e.setAttribute('style','background-color:"

"javascript:" prefix is removed from bookmarklet commands (only
DARKEN used this prefix).
@@ -26,11 +26,7 @@ By default, this mode does nothing but expose the default bookmarklet commands."
`(let* ((source ,source)
(source (etypecase source
(pathname (files:content (make-instance 'files:file :base-path source)))
(string source)))
(source (if (str:starts-with-p "javascript:" source)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather retain the javascript: checking, even if without the quri part. Care to get it back?

The proper implementation, in my opinion, is to strip the scheme and to decode the URL.

@shamazmazum
Copy link
Contributor Author

shamazmazum commented May 27, 2023

@aartaka OK, I'll do it, but I'am very busy at university right now, so my nyxt hacking will have to wait (mostly) :(
Or if you have time for this task, you can do it yourself, I don't really mind :)

@aartaka aartaka closed this in f073f7d May 29, 2023
@aartaka
Copy link
Contributor

aartaka commented May 29, 2023

Fix pushed as f073f7d, thanks for suggesting this change!

aadcg pushed a commit that referenced this pull request Jun 12, 2023
Note that this unreliability only applies to bookmarks, because they have a very
relaxed format. Regular javascript: links are much more likely to have a proper
QURI-parseable format. Thus the fix locality.

Closes #2971.
@aadcg aadcg mentioned this pull request Aug 18, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants