FinalsClub / karmaworld

KarmaNotes.org v3.0
GNU Affero General Public License v3.0
7 stars 6 forks source link

All note links are opened in a new window, even chapter links #296

Closed btbonval closed 9 years ago

btbonval commented 10 years ago

All ANCHOR tags are set to target=_blank so that they open in a new window. https://github.com/FinalsClub/karmaworld/blob/6fb64b36e92f52579ca5b310638f36d66cba35b1/karmaworld/apps/notes/models.py#L304-L313

PDFs frequently have links to their own content on the same page. pdf2html converts these links into ANCHOR tags. Unfortunately the code above makes those ANCHORs also have target=_blank, so that clicking on a self-referencing link opens the same document in a new window.

The following modifications would be better:

  1. only add target=_blank to ANCHOR tags with a non-empty HREF attribute.
  2. only add target=_blank to ANCHOR tags where the HREF attribute does not point at the current page (one that starts with #).

Here is an example of a document with links to self which shouldn't open up in a new window (but do), scroll down to the table of contents on page 2 and click "1.1 Background" for example. http://beta.karmanotes.org/harvard/reason-and-faith-in-the-west/certificate-path-validation-testingpdf-1-17-319897

Here is an example of a document with links which properly open up in a new window, scroll down to the second from last page and click the link in the middle of the page: http://www.karmanotes.org/massachusetts-institute-of-technology/organic-chemistry-ii-133/04_lec_handoutpdf

JHilker commented 10 years ago

William will take this ticket.

btbonval commented 10 years ago

Hey there's a name for this sort of thing. https://en.wikipedia.org/wiki/Fragment_identifier

btbonval commented 10 years ago

@AndrewMagliozzi @william-bratches what is the status of this ticket?

Jacob said William would cover it, and that's the last comment of merit in here. Is this done and merged in?

AndrewMagliozzi commented 10 years ago

Not done to my knowledge

On June 12, 2014, at 11:26 PM, Bryan Bonvallet notificationsd@github.com wrote:

@AndrewMagliozzi @william-bratches what is the status of this ticket?

Jacob said William would cover it, and that's the last comment of merit in here. Is this done and merged in?

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

william-bratches commented 10 years ago

I worked on this several months ago, but it doesn't look like the fix was merged to the current build - I think at the time I didn't have push permission to FinalsClub. I'll push it again this weekend.

btbonval commented 9 years ago

I don't see any commits from @william-bratches after his comment on June 20, 2014 :(

It might have been pushed to a branch, but definitely not in master.

btbonval commented 9 years ago

Only 3 branches off master, none contain anything resembling this :(

btbonval commented 9 years ago

It looks like the sanitizer written by @yourcelf takes care of opening links in a new window. https://github.com/FinalsClub/karmaworld/blob/e8f3f55ff7c1d85430acdc2614081945fe3b61c3/karmaworld/apps/notes/sanitizer.py

Since all notes have been brought back into the database and sanitized, all notes should have properly functioning links in beta and production.

I found some old notes with links on production that open in a new window. I haven't found any newer notes with such links. We can always reopen the ticket if it turns out this is not taken care of.