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

"Open in New tab" does not work with local links (in Jquery) #678

Closed
kelson42 opened this issue Nov 24, 2020 · 12 comments
Closed

"Open in New tab" does not work with local links (in Jquery) #678

kelson42 opened this issue Nov 24, 2020 · 12 comments
Assignees
Milestone

Comments

@kelson42
Copy link
Collaborator

Nothing is loaded, not even the Kiwix extension.

I'm not sure if this is possible at all.

But if not possible we should probably not open a new tab for local links

@Jaifroid
Copy link
Member

@kelson42 We don't have any logic to decode an article and all assets in a shadow DOM and then transfer the HTML to a new tab. I implemented a "breakout link" in KJSW which transfers a copy of the currently loaded article (including images loaded as data URIs) into a new tab (with no Kiwix chrome) for bookmarking an article or side-by-side comparison, or for better printing. But the extracted HTML and images are not navegable, it is a static copy of the article DOM.

Going further than that would require the ability to track and communicate between tabs, and quite a lot of extra logic to manage tabs.

Are you suggesting we try to disable the built-in browser behaviours for ctrl-click or right-click "open in new tab"?

@kelson42
Copy link
Collaborator Author

I primarly report a behaviour which is not really nice for user (actually this is a report from a user).

I suspect that, if we can open a ZIM file without going through the "open file" dialog (which is #656 if I'm right), then we should be in position to open a new Kiwix-JS an open the right ZIM file at the right place. There is as well an obvious "problem" around the URLs to manage.

If that would really not work we might think about catching the click event via JQuery or SW. But again, would be sad to have to follow that path.

@Jaifroid
Copy link
Member

OK, I'll investigate.

@Jaifroid Jaifroid self-assigned this Nov 24, 2020
@Jaifroid Jaifroid added this to the v3.2 milestone Nov 24, 2020
@Jaifroid
Copy link
Member

Jaifroid commented Nov 24, 2020

@kelson42 I've investigated a bit, and it is technically possible to open a new tab using something like:

var newWindow = window.open("article.html", "KiwixTab");
newWindow.onload = function {
    var newWindowDoc = newWindow.document.documentElement;
    newWindowDoc.innerHTML= htmlArticle;
    parseAnchorsJQuery();
    loadImagesJQuery();
    loadCSSJQuery();
    ...
}

The newWindow can be manipulated from the main window in exactly the same way as we manipulate the iframe. It is possible to navigate in newWindow (by clicking on links), and main window will supply the new document, images and CSS, or from main window by using Random button.

I pushed a demo branch of this working pretty well in Chromium and Firefox (jQuery mode) (for Firefox, the user must explicitly allow the "popup" tab). It's very hacky (it closes tab and re-opens it for each new article, just as a kludge to make it work). It does not (currently) work at all in IE11, and works poorly in Edge Legacy (styles and images are not loaded).

See:

And below a sequence in action (look at tabs at top of window):

open_in_tab

@kelson42
Copy link
Collaborator Author

kelson42 commented Nov 26, 2020

@Jaifroid Thank you very much for moving so quickly on this. I would like to step-back for a moment and ask: why exactly this just not work out of the box? Is this because #656? Do we have another problem? We should be carefull to use as much as possible the features of the browser and not mimic them in our own code base.

@Jaifroid
Copy link
Member

See my comment in #679. Opening in a new tab works fine and "naturally" with no kludge in Service Worker mode. My kludge above applies only to the legacy jQuery mode (and sorry for not explaining that!). But I'm glad I've discovered that it is really easy to do in the legacy mode as well!

@kelson42 kelson42 changed the title "Open in New tab" does not work with local links "Open in New tab" does not work with local links (in Jquery) Nov 26, 2020
@kelson42
Copy link
Collaborator Author

@Jaifroid Oh! Here as well indeed... which is a good news! I have renamed this ticket as it was confusing. Your demo looks promising if you can properly attach this new primitive to the even of opening in a new tab (it exists many ways to open a link in a new tab).

@Jaifroid
Copy link
Member

Jaifroid commented Nov 26, 2020

Info to myself: it is possible to trap the contextmenu event, but not possible to know which option a user clicked. However the alternative of Ctrl-click or Command-click can be trapped more reliably, so it would probably be best to use that method and educate users how to open a new tab in jQuery mode, rather than try to simulate the context menu in JavaScript.

More info here: https://stackoverflow.com/questions/8927208/catching-event-when-following-a-link

Also some discussion about replacing context menu in #521, where it was decided it was not a great idea.

@mossroy
Copy link
Contributor

mossroy commented Nov 29, 2020

I don't think there's a clean and universal way to trap the opening of a link in a new tab.
As far as I know, there's no such javascript event that could be trapped.

There are many ways to open a link in a new tab, and some depend on the browser/version/settings. I personally do it with the middle-click of my mouse.
It can also be done with the right-click menu (and I still believe we should not try to override it).
And I suppose we'll also want to do the same for opening in a new window.

Is it worth adding quirks in our code to only partially handle this in jQuery mode?

@Jaifroid
Copy link
Member

Jaifroid commented Nov 29, 2020

I added middle-click support to #680 ;-) (not tested yet -- my other machine has middle mouse button, not the one I'm on right now).

I think #680 addresses three potential issues with the app in its current state:

  • Lack of ability to open a new tab for users of Firefox Extension (or anyone stuck in jQuery mode) - it's not the first time we've been asked for this;
  • Lack of any way to print an article for the same group of users (with Send article to new tab in jQuery mode #680 the user can open a new tab and print from there);
  • Lack of a way to view an article without the Kiwix JS UI (which is not very streamlined yet) - again, we've been asked for this more than once.

Whether it's worth it or not depends on how much of an anomaly it is not to support it. The user who reported this to @kelson42 clearly felt it was enough of an issue to be worth reporting. The lack of SW mode in the Firefox browser extension maybe makes this more of a concern than it would otherwise be.

One possibility (that would also educate users on the existence of the function) would be to have a switch in Configuration:

  • Right-click opens ZIM links in new tab (you can also use ctrl-click, middle-click or long press)

This option would turn the feature on in jQuery mode and suppress the context menu (and would do nothing in SW mode).

@mossroy
Copy link
Contributor

mossroy commented Nov 30, 2020

OK you convinced me.
The configuration wording should say that it's only for jQuery mode, and that it natively works in SW mode.
I don't think we should override the right-click BTW, but only ctrl-click and other ones

@Jaifroid
Copy link
Member

I think we abandoned the PR which might have implemented this. So closing for now.

@Jaifroid Jaifroid modified the milestones: Backlog, v3.8 Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants