Click on a faulty link generates some error and tracebacks #2256
Comments
|
First Analysis Obvously Zim splits the link at the '#': In the case of the problematic anchor link Zim removes all unwanted characters, including The fault seems to be the disregard of the fact, that an anchor part of the link exists, that would result on an anchor link on the current page. This rises the question how to handle faulty links inserted by purpose like This type of link has the same evidence as my original issue. I personally would not call the test |
|
Thanks for the very thorough bug analysis! I encountered this issue before in #1794, where I suggested the following patch to cover up the ValueError, but as you've pointed out, this doesn't cover the hover action. diff --git a/zim/gui/pageview/__init__.py b/zim/gui/pageview/__init__.py
index 83c63c42..57a67568 100644
--- a/zim/gui/pageview/__init__.py
+++ b/zim/gui/pageview/__init__.py
@@ -6511,6 +6511,8 @@ class PageView(GSignalEmitterMixin, Gtk.VBox):
def do_activate_link(self, link, hints):
try:
self._do_activate_link(link, hints)
+ except ValueError:
+ ErrorDialog(self, _('Invalid link: "%s"') % link).run() # T: error when invalid link clicked
except:
zim.errors.exception_handler(
'Exception during activate-link(%r)' % ((link, hints),))
See #1942 - "We need something akin to a HRef.makeValidHRef method as well as a validator for paths." Related issue: #965 |
|
Hi introt, Exchanging the error dialog with traceback by an error dialog might look nicer, but the problem persists. I agree with the #1942 that the whole path/url recognition is involved. So just making a quick'n dirty patch for this one point could break even more at the other hand. To get a new validator for this would be great and. By the way, I would be happy if things like "file:///my/path/with spaces.txt" and similar would autolink correctly... Is there any dev info besides the source code? |
|
Hi,
Besides what's mentioned in the README, not much I'm afraid. You can build the API docs with The Tags and Anchors blueprint might have some relevant info: https://github.com/zim-desktop-wiki/zim-desktop-wiki/wiki/BlueprintTagsAndAnchors As well as the comments on this PR: #1847 Hope this helps, happy hacking! |
|
Agree that in this case the whitespace should be ignored. Tempted to just put a More complex fix would be to put the same logic in the wiki parser and in relevant parts of the PageView such that the whitespace is never handed to the Notes: also put dialog wrapper in place as suggested above for other cases where the name really is invalid |
|
Found a precedent in the commonmark spec, so leaning towards just applying |
Yes. Create the file in /TEMP/ : Doing a drag'n drop to Zim results in this view: Please note that the space (front and end) is shown correctly. Unfortunatelly some special characters are substituted in the user's editor view. Doing a copy/paste from Zim to elsewhere: The source of the Zim page: A doubleclick to this link in Zim will try to open it:
NOTE: In Linux, the extension of the filename is not relevant in most cases. These examples have been tested without extension. Comment Usually I handle Zim notebook content in the editor view (almost never I'm using the HTML-server). So the limitations of local links in a web view are not relevant for my usage case. But having an easy way to integrate files is relevant.
=> Using My suggestion is still
|
|
To start with the `strip()` should happen when parsing wiki links, not
necessarily for all paths / URIs.
Whitespace in the middle of a filename is a very common case indeed.
However, a file path starting or ending with withspace is a special case in
my mind that may require some escaping or character encoding to get it to
work. By comparison, markdown parsing will also drop whitespace at the
start or end of a URI.
…-- Jaap
On Wed, Jan 11, 2023 at 2:45 AM dodrg ***@***.***> wrote:
Could there be any use case where a link actually starts or ends with
whitespace?
Yes.
In Linux spaces, new lines and arbitary signs like *?+ are allowed as
filenames.
Doing a drag 'n drop in Linux from the file manager to Zim will create a
valid link.
Create the file in /TEMP/ :
echo "My new file" > ' leading space, some specials *?+; and a
new line with trailing space '
Doing a drag'n drop to Zim results in this view:
file:///srv/data/TEMP/ leading space%2C some specials %2A%3F%2B%3B and a%0Anew line with trailing space
Please note that the space (front and end) is shown correctly.
Unfortunatelly some special characters are substituted in the user's editor
view.
Doing a copy/paste from Zim to elsewhere:
file:///TEMP/%20leading%20space%2C%20some%20specials%20%2A%3F%2B%3B%20and%20a%0Anew%20line%20with%20trailing%20space%20
The source of the Zim page:
[[file:///TEMP/%20leading%20space%0Aand%20new%20line%20%2B%20trailing%20space%20|''file:///TEMP/ leading space%0Aand new line %2B trailing space'' ]]
file:///TEMP/%20leading%20space%2C%20some%20specials%20%2A%3F%2B%3B%20and%20a%0Anew%20line%20with%20trailing%20space%20
A doubleclick to this link in Zim will try to open it:
- If the content is a text file, it will be opened in the text editor.
- If the content is some HTML, it will be opened by the web browser.
- If the content is an executable (and the x-bit is set) it will fail.
DEBUG: Activate link: file:///TEMP/ leading space%2C some specials %2A%3F%2B%3B and a%0Anew line with trailing space
DEBUG: open_file(/TEMP/ leading space, some specials *?+; and a
new line with trailing space , None)
INFO: Spawning: ('xdg-open', 'file:///TEMP/%20leading%20space%2C%20some%20specials%20%2A%3F%2B%3B%20and%20a%0Anew%20line%20with%20trailing%20space%20') (cwd: /home/drg)
DEBUG: Process started with PID: 940378
gio: file:///TEMP/%20leading%20space%2C%20some%20specials%20%2A%3F%2B%3B%20and%20a%0Anew%20line%20with%20trailing%20space%20: No application is registered as handling this file
DEBUG: Running ErrorDialog
DEBUG: Could not open: file:///TEMP/%20leading%20space%2C%20some%20specials%20%2A%3F%2B%3B%20and%20a%0Anew%20line%20with%20trailing%20space%20
NoneType: None
ERROR: Could not open: file:///TEMP/%20leading%20space%2C%20some%20specials%20%2A%3F%2B%3B%20and%20a%0Anew%20line%20with%20trailing%20space%20
NOTE: In Linux, the extension of the filename is not relevant in most
cases. These examples have been tested without extension.
*Comment*
Usually I handle Zim notebook content in the editor view (almost never I'm
using the HTML-server). So the limitations of local links in a web view are
not relevant for my usage case. But having an easy way to integrate files
is relevant.
- It is nice to have almost full support of the filesystem's
possibilities (spaces, etc.)
- It would be nice to have an even better "pretty view" by having no
substitution signs if not necessary of for Zim.
- Having the choice to opt for copying the file locally to the page
into the structure of the notebook would be great (but is beyond the scope
of this bug report).
=> Using strip() would reduce the filesystem support.
My suggestion is still
- to query the content of the anchor if the URL results to an empty
string.
- This should also be the behavior with a not empty URL.
- If URL and anchor are empty the link attempt should finish silently
with an unlinked result (and tell "nothing to link" in the debug log)
If the user makes nonsense with special characters, IMHO this should be
the users responsibility. In doubt, as the export filter to html / markdown
has to check the code at conversion time, a restrictive link verification
at edit time is to early in the production process, as all tests can be
circumvented by source editing and Zim cannot know the final aims of the
user. So only to warrant the save operation of Zim itself should preferably
set the limits for the content.
—
Reply to this email directly, view it on GitHub
<#2256 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJQYHRKXX3OHWZCFA7YSJTWRYGEPANCNFSM6AAAAAATCJ4F2Q>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Description
Somehow it happend that Zim's automatic link-generation produced a faulty link with the resulting source code:
[[ #zim-pageview| ''#zim-pageview'']]Simplifying the faulty link:
[[ #test]]Please note the leading space before the hash in the link part of the code.
Reproduction
Copy this to the source code of a Zim page to reproduce the problem.
Annotation
I know this is not a valid link and should not appear. But I managed it somehow that the auto-linking of Zim produced the first link while copy/pasting the verbatim marked text. This shall be an issue for another bug report, as currently I cannot reproduce it.
The reported and reproducible issue is that the handling of the invalid link generates an error dialog with a traceback.
Commented Debug Information
Handling the faulty link resulted in more errors and tracebacks as it has been visible to me at the first glance:
This is the debug output of the hover action:
The debug output for the left-click:
The error dialog contained the identical traceback and additionally these details:
Some more details to the system with the error:
[Cut|Copy|Paste|Paste As Verbatim|Delete|<Separator>|Select all|Insert Emoji]Missing are all the link-specific entries:
[Open|Open in a new Window|<Separator>|Cut Link|Copy Link|Edit Link|Remove Link]Also some entries are missing that are available for regular text and links:
[Copy as ...|Copy link to this location|Move selected Text ...]This debug output appears when doing the right-click:
Comments
I would not have expected that an invalid link results in an error but Zim will correct the code smoothly as also done in other parts of the software.
I would see the algorithm that decides to automatically convert a string into a link item in the task to also check the validity of a link item (assuring a consistent handling of links).
In contrast to the faulty anchor link a http link with the same leading space will not fail:
[[ http://test.net/path|http://test.net/path]]Instead of opening the web browser this faulty link will create the new Zim pages
But it will not raise an error. So there the code is correct, even though I'm not happy with the result.
The text was updated successfully, but these errors were encountered: