After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 627060 - double trigger of links
double trigger of links
Status: RESOLVED FIXED
Product: tomboy
Classification: Applications
Component: General
1.3.x
Other Windows
: Normal normal
: ---
Assigned To: Tomboy Maintainers
Tomboy Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-08-16 16:29 UTC by bugra
Modified: 2011-08-20 21:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Disallow links inside of links (2.09 KB, patch)
2010-08-16 18:01 UTC, Benjamin Podszun
accepted-commit_now Details | Review
Modified version of Benjamin's patch (1.93 KB, patch)
2011-06-10 21:23 UTC, Aaron D Borden
committed Details | Review

Description bugra 2010-08-16 16:29:50 UTC
lets say i have a note with following link in it:
http://host/dir/VeryLongDirectoryNameWithWikiWords.html

because of the http prefix the link is already recognized as external HTTP link, but in addition the part with CamelCase (WikiWords) is also recognized as link since in the preferences dialog "Highlight WikiWords" is enabled.

the annoying/buggy part is that if you click on the link somewhere outside the WikiWords then your browser will be opened with the HTTP link as expected; but if you click on the link somewhere on WikiWords then a new note will be created an also your browser will be opened with the HTTP link.
This means there is another link (WikiWords) nested in an already existing link (HTTP).

An idea how this could be solved is to check for longest link (my favorite) or prioritizing links (e.g. HTTP is higher then WikiWords).
Comment 1 Sandy Armstrong 2010-08-16 16:37:06 UTC
Can't test right now, but this is probably not a Windows-specific bug.

This sounds like something that should be fixed ASAP.
Comment 2 Stefan Schweizer 2010-08-16 16:43:11 UTC
Something similar in bug #516636.
Comment 3 Benjamin Podszun 2010-08-16 16:49:06 UTC
Something similar in bug #561456.

ASAP, as in _tonight_?
Comment 4 Benjamin Podszun 2010-08-16 18:01:29 UTC
Created attachment 167988 [details] [review]
Disallow links inside of links

Cause: We don't check if we're inside of a link before creating a new one (or - a broken one).

This bug is affected in two ways: We highlight the wikiword as broken (as in: Create a note if you click on it) link without checking for already present links.

Note links, on the other hand, are clever enough to _not_ create a link inside of a URL link, so this wikiword is going to be a <link:broken> forever.

The patch is a quick one to gather some feedback. It works for me (solving this bug and bug #561456), but this can be done in a bunch of different ways. Adding a new class is probably not the way to go.

So - suggestions? Feedback? Especially Sandy/Stefan? Move to a utility function somewhere (well - where..)?
Comment 5 Sandy Armstrong 2010-08-16 21:07:38 UTC
Review of attachment 167988 [details] [review]:

This looks like a fine solution to me.

BTW, since I have no control over what any contributor does, by "ASAP" I just meant "whenever somebody can do it, this would be a great bug to prioritize".

However, since a fix appeared rapidly after I used that acronym, I'll have to endeavor to use it more in the future!
Comment 6 Aaron D Borden 2011-06-10 20:50:18 UTC
I agree with Benjamin that a class is not the way to go. Let's move this into NoteTag class as HasLinkTag(Gtk.TextIter).
Comment 7 Aaron D Borden 2011-06-10 21:23:42 UTC
Created attachment 189681 [details] [review]
Modified version of Benjamin's patch

Moved the function to the TagTable.
Comment 8 Sandy Armstrong 2011-08-06 06:01:56 UTC
Review of attachment 189681 [details] [review]:

Excellent.

::: Tomboy/Watchers.cs
@@ +900,3 @@
 
+				if (Note.TagTable.HasLinkTag (start_cpy))
+					return;

Not a big fan of early returns, but I don't have the actual code open right now so I'll let you be the judge.
Comment 9 Aaron D Borden 2011-08-20 21:38:36 UTC
(In reply to comment #8)
> Review of attachment 189681 [details] [review]:
> 
> Excellent.
> 
> ::: Tomboy/Watchers.cs
> @@ +900,3 @@
> 
> +                if (Note.TagTable.HasLinkTag (start_cpy))
> +                    return;
> 
> Not a big fan of early returns, but I don't have the actual code open right now
> so I'll let you be the judge.

really? i think early returns are a lot cleaner, as long as they are near the top of the function. Anyway, I changed it to a break, since it is meant to break out of the loop. The loop is the last statement in the function so return and break are equivalent in this case.
Comment 10 Aaron D Borden 2011-08-20 21:39:33 UTC
Comment on attachment 189681 [details] [review]
Modified version of Benjamin's patch

committed as 77dc2ab38bedc0f3ef067dfda5431e8cc93bddaf

This will be included in Tomboy 1.7.4

Thanks for reporting!