search

skeeto / impatient-mode

Impatient html mode. See your changes in the browser as you type
215 stars 19 forks source link

Consider URLs which already have '?' on <link>s "href"s. #3

Closed renatofdds closed 2 years ago

skeeto commented 9 years ago

Hmmm, you've definitely found a bug, but I don't think your patch is the proper solution for it. The question mark should already be encoded and handled as such all the way through this part of the pipeline.

Can you tell me the steps to arrive at a broken impatient-mode page? I just noticed that imp-visit-buffer gets it wrong, and I fixed that in my referenced commit. If you visit from the main listing (/imp/) it already works properly as far as I can tell, since the href is encoded correctly. (This was also a bug at some point.)

Note: MELPA actually pulls from netguy204's (Brian Taylor's) repository, so none of these fixes will actually happen until they arrive in his master. Once we have this issue settled I'll forward the final patch on to him.

Thanks for taking the time to develop a fix, even if we don't use it!

renatofdds commented 9 years ago

I see, but i don't think the question mark should be encoded because i think it might break the functionality. But, anyway, it was just a quick fix just to get it working, i might be completely wrong. Please do what you think it should be done.

To reproduce it simply include a google quick use css font in a head tag, like so:

<link href='http://fonts.googleapis.com/css?family=Open+Sans' rel='stylesheet' type='text/css'>

When the page refreshes from a signal of impatient mode in the long poll it tries to load it again, this time with two '?' (with the id appended), which fails.

Thanks already for looking into it!

Christopher Wellons notifications@github.com writes:

Hmmm, you've definitely found a bug, but I don't think your patch is the proper solution for it. The question mark should already be encoded and handled as such all the way through this part of the pipeline.

Can you tell me the steps to arrive at a broken impatient-mode page? I just noticed that imp-visit-buffer gets it wrong, and I fixed that in my referenced commit. If you visit from the main listing (/imp/) it already works properly as far as I can tell, since the href is encoded correctly. (This was also a bug at some point.)

Note: MELPA actually pulls from netguy204's (Brian Taylor's) repository, so none of these fixes will actually happen until they arrive in his master. Once we have this issue settled I'll forward the final patch on to him.

Thanks for taking the time to develop a fix, even if we don't use it!

Reply to this email directly or view it on GitHub.*

netguy204 commented 9 years ago

Seems like a reasonable enough approach to me. Thanks for the patch!

skeeto commented 9 years ago

Ah, I understand now. I thought it was a question mark in the buffer name that was a problem (it was, but that's unrelated to this). The problem is that it shouldn't be appending that parameter to every link tag, just the ones hosted by impatient-mode itself.

I just made a commit to fix this (also referencing this issue).