553132 - URLs entered in the task description should be clickable and open a browser

Status: NEW

(Calendar :: Tasks, enhancement)

Description

[Carl Menezes]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2) Gecko/20100115 Firefox/3.6 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.7) Gecko/20100111 Lightning/1.0b1 Thunderbird/3.0.1

If you enter URLs in a task's description, they are not clickable and they should be. 

Reproducible: Always

Steps to Reproduce:
1.Create a new task
2.Add information to it in the description, with one or more URLs.
3.You cannot click on the URL to go to the page
Actual Results:  
The URL appears as plain text.

Expected Results:  
The URL appears as a link that can be clicked on. 

I know a URL can be attached to the task. The problem is that the attachment does not contextual information describing the URL. That might be ok for people used to tools like bugzilla, but I doubt that the majority of users are. 

These days, one simply expects to be able to click on links.

Comment 1

[sven.nv+bugzilla]

I'm not familiar with calenders code, but i guess it should be fixed in there:

http://hg.mozilla.org/comm-central/file/84961292898a/calendar/base/content/calendar-task-view.js

Comment 2

[Lewis Rosenthal]

More than just the description field should autolinkify. Consider urls entered for event locations.

Comment 3

[Lewis Rosenthal]

Cross-reference: bug 734072.

Comment 4

[Manu Jain]

Just want to know that if someone is still working on the bug or not? If not then I'll like to take this bug up.

Comment 5

[Lewis Rosenthal]

I always wanted to find some time to play with this, but haven't gotten myself around to it (always something else). I'm happy to test (Linux, Windows, OS/2), if you're willing to code, Manu!

Comment 6

[Philipp Kewisch [:Fallen][☁️📆]]

Feel free to work on this bug :-) Let me know if you have questions!

Comment 7

[Manu Jain]

My idea in this bug is to use regular expressions to detect URL in the task description and then return that URL as a href link. Is it a good approach or do I need to do something different?

Comment 8

[M Lopez-Ibanez]

(In reply to Manu Jain from comment #7)
> My idea in this bug is to use regular expressions to detect URL in the task
> description and then return that URL as a href link. Is it a good approach
> or do I need to do something different?

Thunderbird is already doing this for plain-text emails, thus there must be code somewhere to do precisely this.

The add-on Thunderbird Conversations (https://github.com/protz/GMail-Conversation-View/) is also doing it, and I guess the code must be somewhere here: https://github.com/protz/GMail-Conversation-View/blob/master/modules/message.js You could always ask the author.

Comment 9

[Manu Jain]

Attachment: bug_553132.patch

Patch attached.

Comment 10

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 8564599 [details] [diff] [review]
bug_553132.patch

Review of attachment 8564599 [details] [diff] [review]:
-----------------------------------------------------------------

Codewise this looks fine with a few minor comments, but we'll need to give it a test run. Matt, if you get to that before me feel free. Manu, do you think you could upload a new patch with these comments fixed?

::: calendar/base/content/calendar-task-view.js
@@ +121,5 @@
>                      rpv.value = detailsString.split("\n").join(" ");
>                  }
>              }
> +            var texttohtml = Components.classes["@mozilla.org/txttohtmlconv;1"]
> +                    .getService(Components.interfaces.mozITXTToHTMLConv);

Just a minor indentation issue here, make sure you are using spaces not tabs and align the . before getService with the . after Components. w

@@ +122,5 @@
>                  }
>              }
> +            var texttohtml = Components.classes["@mozilla.org/txttohtmlconv;1"]
> +                    .getService(Components.interfaces.mozITXTToHTMLConv);
> +            var textbox = document.getElementById("calendar-task-details-description").contentDocument;

Its not a textbox anymore, do you think you could rename this to descDocument ? Also, please use let instead of var, for all variables you've added and maybe also the remaining ones just around the code you've changed, i.e. |description|.

::: calendar/base/content/calendar-task-view.xul
@@ +221,5 @@
>                  </row>
>                </rows>
>              </grid>
>            </hbox>
> +          <iframe src="about:blank" id="calendar-task-details-description" multiline="true" flex="1"/>

You should add type="content" here to make sure the content cannot access privileged apis.

Comment 11

[Manu Jain]

Attachment: bug_553132_v2.patch

changes made.

Comment 12

[Matthew Mecca [:mmecca]]

Comment on attachment 8564604 [details] [diff] [review]
bug_553132_v2.patch

Review of attachment 8564604 [details] [diff] [review]:
-----------------------------------------------------------------

r- on this version of the patch, overall it's looking good, but has a few issues:

- Selecting a task without a Description set throws an error, and the description frame doesn't update from the value for the previously selected task.

- Clicking on a link loads the web page in the description frame rather than opening it in a new browser window.

Comment 13

[Manu Jain]

I have corrected the first issue.
But I have used iframe in the xul file of task-view and iframe opens the link in that frame itself. I have used iframe because textbox (which is earlier used) doesn't support href links. So, can you tell how to open those links on a separate browser?

Comment 14

[Manu Jain]

Attachment: bug_553132_v3.patch

First issue fixed.
In second issue rather than opening in new window, its opening in new tab of that window only. I think that is fine. Please take a look.

Comment 15

[Matthew Mecca [:mmecca]]

(In reply to Manu Jain from comment #14)
> Created attachment 8565926 [details] [diff] [review]
> bug_553132_v3.patch
> 
> First issue fixed.
> In second issue rather than opening in new window, its opening in new tab of
> that window only. I think that is fine. Please take a look.

I think it would be more consistant with how Thunderbird handles linkified emails if they opened in a browser window rather than a tab. 

You should be able to use the same function as the message pane uses [1]. You'll need to add a line to the scripts section of calendar-task-view.xul:

<script type="application/javascript" src="chrome://communicator/content/contentAreaClick.js"/>

Then in your iframe tag you can add:

onclick="return contentAreaClick(event);"



[1] http://mxr.mozilla.org/comm-central/source/mail/base/content/messenger.xul#583

Comment 16

[Stefan Sitter [:ssitter]]

And please don't forget to test and verify the feature in SeaMonkey too because you don't want to break Lightning for all SeaMonkey users.

Comment 17

[Manu Jain]

Attachment: bug_553132_v4.patch

Second issue also fixed.

Comment 18

[Matthew Mecca [:mmecca]]

Comment on attachment 8569048 [details] [diff] [review]
bug_553132_v4.patch

Review of attachment 8569048 [details] [diff] [review]:
-----------------------------------------------------------------

Unfortunately using contentAreaClick as I had suggested doesn't work in SeaMonkey. What we can do instead is add our own event handler to taskDetailsView, something like this:

    onContentAreaClick: function onContentAreaClick(aEvent) {
        if (aEvent.target instanceof HTMLAnchorElement) {
            launchBrowser(aEvent.target.href, aEvent);
        }
    },

and in the iframe use onclick="taskDetailsView.onContentAreaClick(event);"

I briefly tested this in Thunderbird and SeaMonkey and it seams to work as expected, but please test.

Also, another issue I found is that line break formatting is lost if the task's description has multiple lines. We should be able to fix that by doing something like

 description = texttohtml.scanTXT(description, Components.interfaces.mozITXTToHTMLConv.kURLs)
                         .replace(/\n/g, "<br>");

but again, I only tested that briefly.

Comment 19

[Manu Jain]

Attachment: bug_553132_v5.patch

Tested on thunderbird. Works fine.

Comment 20

[Matthew Mecca [:mmecca]]

Comment on attachment 8571001 [details] [diff] [review]
bug_553132_v5.patch

Looks good to me, pending ui-review.

Richard, do you think we need any style changes? It looks like the font changed a bit with the change to an iframe.

Comment 21

[Richard Marti (:Paenglab)]

Comment on attachment 8571001 [details] [diff] [review]
bug_553132_v5.patch

Yes, the font changed as it's now like a web page. It follows now the setting in Options/Display, also with color/background color. With this change it behaves now more like a normal message.

Now the font-family and font-size can be removed in https://dxr.mozilla.org/comm-central/source/calendar/base/themes/common/calendar-task-view.css#31 because they are now no more working.

Does already a bug exist to make the URLs clickable in the event description? I think this would make more sense than the actual approach with the URL as attachment. And use the attachments for documents etc. (but this would surely be an other beast to implement).

Comment 22

[Richard Marti (:Paenglab)]

Comment on attachment 8571001 [details] [diff] [review]
bug_553132_v5.patch

Sorry to change the ui-r, but I've seen there is no context menu for copy/paste/etc.

Comment 23

[Matthew Mecca [:mmecca]]

Comment on attachment 8571001 [details] [diff] [review]
bug_553132_v5.patch

Review of attachment 8571001 [details] [diff] [review]:
-----------------------------------------------------------------

Manu, could you add a context menu for the iframe as Richard suggested? It should only need a few menu items since we're not supporting inline editing at this point.

Comment 24

[M Lopez-Ibanez]

bug 143475 would be a superset of this.

Comment 25

[Christian Weiske]

Calendar event descriptions also do not have clickable links.

Comment 26

[pgnet.dev]

The error rate for end-users incorrectly copy-n-pasting commonly long, but TXT format, vidcall links is, here, unacceptably high.

Ppl are missing meetings left & right ...

Some services pop up an error when an incorrectl/nonexistent vid-link is attempted; others, like Jitsi, will happily create a mtg room for any link your create.

In current state, it's really a non-starter for wide rollout/use.

Comment 27

[foppejulian]

Is there any update to this topic?
In the time of corona, i have a lot of virtual Meetings and i have to open and edit every meeting i want to copy the URL from.
So if the descripption-popup would be permanent if you hover over it and links be clickable it would a big relief.

Comment 28

[remi sharrock]

Very interesting topic indeed. I didn't know this part of Thunderbird was written in Javascript :)
I wish I had time to take the source code, know how mercurial is working, look into https://hg.mozilla.org/comm-central/file/5b69deb258bec457a5fd4f0df4b1c0a443e9c543/calendar/base/content (if it were to be here) and try some things to implement it... Maybe I can ask my students to give it a try !

Comment 29

[Vousk-prod.]

May this issue a duplicate of
https://bugzilla.mozilla.org/show_bug.cgi?id=697370 ?