390753 - Event Summary Dialog: Copy is not possible (no context menu or selection)

Status: RESOLVED FIXED

(Calendar :: Dialogs, defect)

Description

[Christian Jansen]

* Create an event,
  * put some text into the Description field
* Switch the calendar to Read Only
* reopen the event 
* Select the text you wrote in the Description field
* Context click
* Select Copy.

Result: Clip Board is empty
Expected: Clip Board should be filled with the selected text.

Comment 1

[Pete Riley]

I can reproduce this bug with Lightning 2007-10-01.  However, it works if I (1) leave the Event Summary dialog open, (2) switch to another program on my computer, (3) switch back to Thunderbird, and (4) right-click & copy again.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Oops, sorry, my bad, returning to the right component, awfully sorry about stomping your target milestone, please send hatemail to this address :(

Comment 3

[Omar Bajraszewski]

I cannot reproduce it with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.1b2pre) Gecko/20081010 Calendar/1.0pre

Comment 4

[Martin Schröder [:mschroeder]]

Confirmed using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b2pre) Gecko/20081103 Lightning/1.0pre Shredder/3.0b1pre

Can't bring up the context menu using right click, and also no selection is possible.

Comment 5

[Anup Kumar [:reznord]]

I'm able to copy the text in the description dialog box. Seems like the bug is fixed with some other patch.  

Can anyone please confirm it once again before marking it as resolved?

Comment 6

[sakshivaid95]

I am unable to reproduce the bug. I think this has been already fixed. Please confirm.

Comment 7

[[:MakeMyDay]]

This is not fully working. Selecting and copying works for the description box, but not for any other content like dates, attendees, url etc. That said, I'm going to reopen this bug.

Comment 8

[Pavan Karthik [:matrixisreal]]

Hi,
the bug is still reproducable,as MakemyDay said everything except description isnt allowing selection.
Please let me know on where i need to look at/make changes(Im new to calendar)

Thanks

Comment 9

[Derk Vedelaar]

Attachment: selectable_calendar_fields.patch

I created this patch which tries to fix this bug.

All the fields in the summary-dialog that were containing information and were a label are now disabled textfields, styled to look like labels. This changes the ui for read-only calendar invites as well as read-only tasks.
This has the effect that you can now select the text and copy it.

It also sets the focus to the "OK" button (which was previously on the first selectable field in the window, otherwise the title would always be selected (and thus be blue) when the dialog is opened.

Unfortunately the "repeat" row cannot be selected all at once, but has to be selected per-line. I was unable to fix that and still have it look good.

Comment 10

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8917446 [details] [diff] [review]
selectable_calendar_fields.patch

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

This looks fantastic, thanks for putting together the patch! I gave it a quick test and for me on mac the textbox background is different from the dialog, so there is a slight shadow. I'd suggest adding { background-color: inherit; color: inherit; } to cancel out the color set for the background.


I have a few minor code comments. I'd appreciate if you could upload a new patch with the mentioned issues fixed, you can then request review from me (autocomplete on :Fallen).

::: calendar/base/content/calendar-item-bindings.xml
@@ +26,4 @@
>        <xul:label anonid="item-datetime-label"
>                   class="headline"
>                   xbl:inherits="align"/>
> +      <xul:textbox readonly="true" 

There are some stray whitespaces at end of line here and in your css file, I'd appreciate if you could remove them.

::: calendar/base/content/dialogs/calendar-summary-dialog.js
@@ +205,4 @@
>      // can't modify anything, thus we go ahead with an 'ok' button only.
>      if (window.readOnly) {
>          document.documentElement.getButton("cancel").setAttribute("collapsed", "true");
> +        document.documentElement.getButton("accept").focus();

Can you see if you can trigger the same behavior by setting the defaultButton attribute? https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/dialog#a-defaultButton

::: calendar/base/content/dialogs/calendar-summary-dialog.xul
@@ +60,2 @@
>              <label value="&read.only.title.label;"/>
> +            <textbox id="item-title" readonly="true" />

Let's be consistent w.r.t. whitespace after last attribute and remove it here...

@@ +82,2 @@
>              <label value="&read.only.location.label;"/>
> +            <textbox id="item-location" readonly="true" />

...and here.

::: calendar/base/themes/common/dialogs/calendar-event-dialog.css
@@ +563,5 @@
> +#calendar-event-summary-dialog .item-datetime-value,
> +#calendar-task-summary-dialog .item-datetime-value,
> +#calendar-summary-dialog .repeat-details-value,
> +#calendar-event-summary-dialog .repeat-details-value,
> +#calendar-task-summary-dialog .repeat-details-value

Here it probably makes sense to give the respective textboxes a class and then change the css rule accordingly.

Can you let me know why a fixed height was needed? It seems to be fine without for me. If it is necessary, it would probably be better to convert it to ex or em units.

Comment 11

[Derk Vedelaar]

Attachment: selectable_calendar_fields_2.patch

> Can you let me know why a fixed height was needed? It seems to be fine without for me.  If it is necessary, it would probably be better to convert it to ex or em units.
In my opinion it looked better. I found a better option though, by adding the "plain" class it looks better than with my fixed height ( https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Style/plain ). 
The declaration of the "plain" class is as following:

> .plain {
>   -moz-appearance: none;
>   margin: 0 !important;
>   border: none; 
>   padding: 0; 
> }

So I removed the -moz-appearance too.

> I gave it a quick test and for me on mac the textbox background is different from the dialog, so there is a slight shadow. I'd suggest adding { background-color: inherit; color: inherit; } to cancel out the color set for the background.
I do not have this problem under linux (ubuntu 16.04), nor do I have a mac to test this. I added your suggestion as it also does not do any harm under linux. Right now I have it still in the common css, but I might as well move it to the osx css files. What is your thought on this?

> There are some stray whitespaces at end of line here and in your css file, I'd appreciate if you could remove them.
Fixed 

> Can you see if you can trigger the same behavior by setting the defaultButton attribute? https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/dialog#a-defaultButton
I tried that. It's not the same. This will make the "enter" button close the window, but will not actually focus the button, which will result in the "title" being focussed and selected. Also by default the "OK" button is the "defaultButton", so there should be no need to set that (source: https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/buttons see "accept" ).

> Let's be consistent w.r.t. whitespace after last attribute and remove it here...
Fixed

> ...and here.
Fixed

> Here it probably makes sense to give the respective textboxes a class and then change the css rule accordingly.
Fixed

Comment 12

[Philipp Kewisch [:Fallen][📱📆]]

Comment on attachment 8918291 [details] [diff] [review]
selectable_calendar_fields_2.patch

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

(In reply to vedelaar from comment #11)
> Created attachment 8918291 [details] [diff] [review]
> selectable_calendar_fields_2.patch
> 
> > Can you let me know why a fixed height was needed? It seems to be fine without for me.  If it is necessary, it would probably be better to convert it to ex or em units.
> In my opinion it looked better. I found a better option though, by adding
> the "plain" class it looks better than with my fixed height
Great, let's go with the plain class!

> > I gave it a quick test and for me on mac the textbox background is different from the dialog, so there is a slight shadow. I'd suggest adding { background-color: inherit; color: inherit; } to cancel out the color set for the background.
> I do not have this problem under linux (ubuntu 16.04), nor do I have a mac
> to test this. I added your suggestion as it also does not do any harm under
> linux. Right now I have it still in the common css, but I might as well move
> it to the osx css files. What is your thought on this?
I think we can go ahead and leave it in common, this isn't specific enough to warrant moving into per-platform files, and it doesn't do any harm on other platforms.

> > Can you see if you can trigger the same behavior by setting the defaultButton attribute? https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/dialog#a-defaultButton
> I tried that. It's not the same. 
Ok, thanks for testing!

Based on that it looks like this patch is ready to go, I'm going to set checkin-needed in a moment, if you want to make it easier for the sheriff you could upload a new patch with patch header with your username set and a commit message like "Bug 390753 - Make it possible to copy text from event summary dialog. r=philipp"

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/cd77c19988de
Make it possible to copy text from event summary dialog. r=philipp