321434 - Location does not appear in multiday view event box

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[gekacheka]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: trunk 20051224

Previously day and week view event boxes contained
  Title (location)
  descriptionLine1
  descriptionLine2...

With new views, the contain only
  Title

Reproducible: Always

Steps to Reproduce:
1. Create a multi-hour event with title, location, and description.
2. view in day view or week view (new views)


Actual Results:  
  Title

(and lots of empty space)

Expected Results:  
  Title (location)
  descriptionLine1
  descriptionLine2...


how old week view added event box text
http://lxr.mozilla.org/mozilla/source/calendar/resources/content/weekView.js#464

Comment 1

[gekacheka]

Attachment: v1. add location and description to multiday-view event box

(patch -l -p 2 -i file.patch)

Separates the display <description> / edit <textbox> into calendar-item-field-textbox, and uses it for the title, location, and description.  Title and location are zero or one line each, wrappable if long.  Description takes the remaining space, wrappable.  Calendar-item-field-textbox has a minimum height of 2 pixels so that if there is no location, it won't take much space, but will still be clickable so a location can be added.

This bug was only a regression in sunbird.  This patch assumes that it is ok to add this feature to lightning as well.  

Adding the additional info probably has a speed tradeoff.  I tried to use the same techniques (use description for display, and hide textbox unless editing), but paired them in a deck.

(One minor issue I haven't been able to resolve:  in day view, a long description paragraph is wrapped to width of the longest word, as if it is originally laid out for a zero-width event box, and not re-wrapped for the final width.  I'm assuming this is a layout bug.  Jiggling the width of the pane causes it to re-wrap correctly.)

Comment 2

[Michiel van Leeuwen (email: mvl+moz@)]

I'm not sure if we really want to show all the info. I'm afraid it will look very crowded, and you won't be able to find the information you are looking for.
And besides, the location and description are (or should be) in the tooltip.
Can you provide screenshots of the day and weekviews? (with some reasonable lenght events, like 1 hour)

Comment 3

[gekacheka]

Attachment: v1 preview: Sample day view with location and description

Comment 4

[gekacheka]

Attachment: v1 preview: Sample week view with location and description

Comment 5

[Simon Paquet [:sipaq]]

In the week view (multiday view) the description should not be shown by default IMO, only the location. The description can become very long and unless you have very long events which do not collide which other events at the same time, you won't see the description most of the time.

I'm fine with it in the day view, because we have much more space there.

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

I think we should only show the location. The description will be too long, and look too cluttered. (Even in day view. There might be more space, but that's only horizontal space. A description of a few lines, and it's very busy again)

Comment 7

[Michiel van Leeuwen (email: mvl+moz@)]

But let's ask beltzner...

Comment 8

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

Yeah, the description info seems like overkill. The location should probably also only be shown on the day view, not the week or month views. Ideally these things will all be skinnable though, right?

Comment 9

[gekacheka]

I can agree the description could be optional.
I think the option should be controlled by the user as an individual preference, not tied to an overall theme.  (There is no such thing as a theme extension, is there?)

Showing location in week view is useful to notice where there might be bad transitions between events that are far apart in space, far enough ahead of time that one might be rescheduled or shortened if necessary.

Comment 10

[Simon Paquet [:sipaq]]

(In reply to comment #9)
> I can agree the description could be optional. I think the option should be 
> controlled by the user as an individual preference, not tied to an overall 
> theme.  (There is no such thing as a theme extension, is there?)

Don't go the pref route! We have seen what will happen when that happens with the Suite. I like the skinning suggestion very much. Let's just add a display:none; for the description in the day and week view CSS so that themers can change that as well as individuals via userchrome.css

> Showing location in week view is useful to notice where there might be bad
> transitions between events that are far apart in space, far enough ahead of
> time that one might be rescheduled or shortened if necessary.

I tend to agree with that suggestion, though I'm not the UI expert here.

Comment 11

[Stefan Sitter [:ssitter]]

*** Bug 330390 has been marked as a duplicate of this bug. ***

Comment 12

[Reed Loden [:reed]]

The bugspam monkeys have been set free and are feeding on Calendar :: Internal Components. Be afraid for your sanity!

Comment 13

[Michael Büttner (no reviews TFN)]

*** Bug 360909 has been marked as a duplicate of this bug. ***

Comment 14

[Simon Paquet [:sipaq]]

Comment on attachment 206859 [details] [diff] [review]
v1. add location and description to multiday-view event box

Moving first-review to Stefan, since mvl's queue is pretty big and he can only cope with his ui-reviews and 2nd reviews.

Comment 15

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 206859 [details] [diff] [review]
v1. add location and description to multiday-view event box

ui-review=mvl on adding the location. You can add the description, but make is default to hidden using CSS. Then users can enable it (maybe using a very simple extension)

Comment 16

[gekacheka]

Comment on attachment 206859 [details] [diff] [review]
v1. add location and description to multiday-view event box

patch no longer applies, due to many changes to underlying code.

Comment 17

[gekacheka]

Attachment: v2 patch: add location and description to multiday-view item box (refactor item-boxes)

(patch -l -p 2 - i file.patch)

Result:
  In week view, events display Location (if tall enough)
  In Day view, events display Location and Description (if tall enough)
  In both these views, all-day events display location (if wide enough)
These will be more useful once hour height can be enlarged by the user.

Other Bugs fixed:
* Now possible to insert cursor in Month view event titles.
  1. click on event in month view to select event,
  2. click on title of selected event to edit it (becomes selected)
  3. click between characters of selected title to place cursor.
  (click in step 3 previously ended edit).
* Now possible to control-click (command-click on MacOS) to toggle selection.
  Clicking with toggle modifier never starts to edit text.
  Clicking without modifier starts to edit text only if previously selected.
  Double-click and context-click still work.

Minor problems:
* When the location field is empty, it can be difficult to click to
  edit it.  This is partly a layout problem, at least in in all-day
  events in multiday-view, where the empty box has ample min-height
  and min-width, but it is laid out below the baseline from where you
  might expect.  (Starting an edit and exiting repositions the box
  above the baseline, so it does look like a layout bug.)  

  Workaround is to click in the title, then use tab to shift focus to
  the location.  (Or use double click to open the edit dialog.)

* If a field is not edited, it is possible to tab or shift-tab to
  shift focus to next field, but after editing a field, tabbing to
  the next field saves the editted field, which currently causes the
  whole itembox to be deleted and redisplayed.

* On a weekview with 160 15-minute events, redisplay is near 50% slower
  than before this patch.  (See ideas for future below).

Refactoring:

Introduced calendar-item-field-textbox used for each field.
calendar-item-box
  contains the title field textbox.
calendar-month-day-box-item extends calendar-item-box.
  Contains a label for the time and the title field textbox.
calendar-item-timespan-box extends calendar-item-box.
  Contains gripbars and the title, location, and description field texboxes.

Introduced method initializeFromOccurrence so derived bindings can
  extend this method and call the base method to avoid duplication.
  calendar-month-day-box-item extends the method to initialize the
    time label.
  calendar-item-timespan-box extends this method to initialize the
    location and description fields
  (Previously, the derived bindings duplicated the occurrence
  property, copying all the the setter code in the base binding 
  in order to add to it.  Now the setter calls initializeFromOccurrence
  after setting the occurrence.)

Click handling is now split between the
  calendar-item-box, which handles selection and tabbing focus, and
  calendar-item-field-textbox, which handles start/end editing a field.
  Custom FieldEditStarted events and FieldEditStopped events are used to
    notify parents when to start and end edit mode (to ignore clicks).

Simplifying:

calendar-item-timespan-gripbar parentorient attribute, and several
  listeners and methods to keep it up to date, were all removed.
  (It was used only for the CSS cursor, but changing to use
   a CSS rule referencing calendar-item-timespan[orient="..."]
   seems to be just as fast and simplifies the code.)

calendar-month-view.relayout
  removed code that initialized item-box "class" and "tooltip" since that
  duplicates the previous initialization in the item-box constructor.

Renaming:

Renamed calendar-editable-item to calendar-item-box
  (editable parts moved to calendar-item-field-textbox)
Renamed calendar-event-box to calendar-item-timespan-box, 
and calendar-event-gripbar to calendar-item-timespan-gripbar
  to make clearer it is not used in month-view, and that
  it may display items other than events (such as tasks).
  (It was confusing to see closely related bindings named
   with '-item' in month-view and view-core, but '-event' in
   multiday-view.)

Ideas for future:
* speed: don't create editing textbox until it is needed.
* speed: test whether userchrome can override xbl bindings; 
  if so, then create separate versions of multiday-view which 
  creating different day-view and week-view items.
* field: Attendees (some meetings are better identified by attendee rather 
  than title, e.g., tutor/consultant/doctor/dentist/lawyer/hair appts.).

Comment 18

[Matthew (lilmatt) Willis]

While I'm impressed with the refactoring, I'm not really comfortable taking it in 0.5 with so many of those minor issues outstanding.

Perhaps you can continue to refine this to address as many of them as you can?  I also suggest keeping up with mickey's work in bug 368982 so you guys don't duplicate effort (for this bug and your shrinkable/expandable view bug)

Removing review request for now

Comment 21

[gorko122]

how is the actual state of this "bug"?

it has been reported three years ago and I think it isn't so hard do implement.

I would be very proud if you could implement it for the next release.

THANKS

Comment 23

[gekacheka]

Attachment: v3 sanity test data: Mozilla Summit schedule snapshot

Snapshot of schedule of a medium size multi-track symposium/summit (3day, up to 7 parallel sessions: test narrow events).
- Titles, locations, descriptions, and URIs are included (test wrapping and pre-formatted descriptions).  Titles include presenters where available.
- Category is same as location room, so by coloring categories user can see tracks.  (test category marking)
- Dates are 'local/floating' (no timezone) so that it can be used by testers in any timezone (schedule won't break over midnight in any tester timezone)
- a few are marked tentative, confirmed, or cancelled (test status marking)

Comment 24

[gekacheka]

Attachment: v3 preview: Day View of sanity test data (Summit)

Comment 25

[gekacheka]

Attachment: v3 preview: Week View of sanity test data (Summit)

3-day work-week only is shown.

Category colors used in image to show tracks (category is room name):
  Emerald:    turquoise
  Callaghan:  dark green
  Crystal:    light yellow
  Alpine C:   tan
  Alpine D:   light brown
  Alpine E:   dark brown
  Alta:       black
  Glacier:    blue

italic is tentative
bold is confirmed
line-thru is cancelled

Comment 26

[gekacheka]

Attachment: v3 performance test data: 5 days of 15 minute appointments

140 events in one work week

Goal: Advancing to this week from previous week should be about the same speed, not slower (previous patch was slower, took 1.5 times the time).

Comment 27

[gekacheka]

Attachment: v3 manual test script for calendar grid items

Comment 28

[gekacheka]

Attachment: v3 patch: add location and description to multiday-view item box (refactor item-boxes)

Updated patch from comment 17.

New performance change:  textbox for each text field is not created unless needed, so this version is about the same speed as the current version (unlike previous patch), as tested using the performance test data (comment 26).

New refactoring:  calendar-item-frame implements the common elements from month-view-day-box-item, multiday-view calendar-item-allday-box, and multiday-view calendar-item-timespan-box.  These include the border, the background color, the selection color, the background gradient, and the category stripe.

Comment 29

[Simon Paquet [:sipaq]]

Gekacheka, is your latest patch ready for review or are you still working on it?

Comment 30

[gekacheka]

How soon is 0.9, and is this too risky for 0.9? (or too risky to add after 0.9 with 1.0 next?)  

My initial thought was to wait until after 0.9 since 0.9 is behind schedule and this hasn't been flagged on the schedule for 0.9, but I can see maybe it is too big a UI change to introduce in 1.0 without wider than nightly-user testing.  Ideally it would have both a period of nightly-user testing and some-release-before-1.0 user testing.  

(BTW, the patch will no longer apply after some patches for bugs which *are* flagged for 0.9, like bug 437412, so I expect at least one update will be needed.)

Comment 31

[Daniel Boelzle [:dbo]]

I think we should refrain from further UI changes for 0.9 and concentrate on getting the blocking bugs fixed. This is an excellent candidate to land shortly after 0.9 though.

Comment 32

[Matthew Monaco]

Suggestion:

Allow for custom formatting of fields.  %sd = start date, %t = title, etc.  Formatting can be applied to single instances, reoccurring events, calendars, different views, etc.

Comment 34

[moxudo]

(In reply to Matthew Monaco from comment #32)
> Suggestion: Allow for custom formatting of fields. [...]

Agreed. It's very inconvenient having to work-around this bug by adding the event location to the title or even the calender name.

Comment 35

[Clemens]

Hi Community,

This is my First comment, so please go easy on me.
I have included a location field in daily/weekly view:
<Event>
grippyTop
Title(Editable)
Location
empty Space
grippyBottom
</Event>

Added a Checkbox in Preferences Show Location in daily/weekly view.
(I have problems translating the Pref-String).

So far it works and looks good in 3.3.2 and 3.9.1 Lightning-Calendar

I still need a good example of how to translate strings for Preferences, if i just add pref.showlocation.label to dtd it crashes the Preferences View (general in my case). Don't know why :/
Since the translation is the last Problem, i will try to solve that and afterwards upload the files i worked, which are so far:
chrome/calendar/content/calendar/calendar-view-core.xml -> create fields and base layout (box) + property & method

chrome/calendar/content/calendar/calendar-multiday-view.xml -> create fields and base layout (box)

chrome/calendar/content/calendar/calendar-base-view.xml -> observer for Preference

chrome/calendar/content/calendar/preferences/general.xul -> create Preference

chrome/skin/common/calendar-views.css -> Style

Comment 36

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

Hi Clemens,

if with crashing the preferences view you mean the yellow DTD error thing, then its probably because the dtd you added the string in is not part of that dialog or window. You need to make sure you add the dtd entry to a file that is included. Check other strings around it and figure out in which file they are.

This file is a good candidate if your string is in the "views" tab of the preferences.

http://mxr.mozilla.org/comm-central/source/calendar/locales/en-US/chrome/calendar/preferences/views.dtd


When you upload the file changes, can you upload them as a patch against http://hg.mozilla.org/comm-central/ ? Also, for review a screenshot would be nice.

Comment 37

[Clemens]

Attachment: calendar-view-core.xml

hopefully the diff comes automatically

Comment 38

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

Unfortunately the diffs aren't created automatically. If you need help with that please let me know, there is a guide available here:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Comment 39

[Clemens]

Attachment: location_v1.patch

Hello again,

i finally could upload a patch, that is diffed to the actual version (lightning 4.0a1). Check it out, I will add a screenshot too.

Feedback appreciated.

Comment 40

[Clemens]

Attachment: Screenshot of location_v1.patch in action

Comment 41

[moxudo]

(In reply to Clemens from comment #39)
> [...] Feedback appreciated.

It looks very neat and IMHO completely solves the problem :-)
Just 1 small question: In even smaller event boxes, will the location take precedence over the title? IMHO, both should be abbreviated if necessary, rather than not showing any location info at all.

Comment 42

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

Comment on attachment 8567969 [details] [diff] [review]
location_v1.patch

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

A first pass at the review, I have yet to test it. Most are just minor nits, but I'd appreciate if you could upload a new patch with the comments fixed.

::: calendar/base/content/calendar-base-view.xml
@@ +644,5 @@
>                 case "calendar.date.format":
>                     this.refreshView();
>                     break;
> +               case "calendar.view.showLocation":
> +                   this.refreshView();

Since its the same command, you can just have it fall through.

::: calendar/base/content/calendar-multiday-view.xml
@@ +2255,3 @@
>                            anonid="calendar-event-details"
>                            align="start">
>                    <xul:image anonid="item-icon"

This will make the image be above location and summary, wasting space for short one-line events.

@@ +2396,5 @@
> +          var width = document.getAnonymousElementByAttribute(this, "anonid", "eventbox").boxObject.width;
> +          evl.setAttribute("style", "width: " + width + "px;max-height: " + Math.max(0, height-gripbar*2) + "px");
> +          //evl.setAttribute("style", "width: " + width + "px;height:20px");
> +          evloc.setAttribute("style", "width: " + width + "px;max-height: " + Math.max(0, height-gripbar*2) + "px");
> +          //evloc.setAttribute("style", "width: " + width + "px;height:20px");

Please remove the commented out lines.

::: calendar/base/content/calendar-view-core.xml
@@ +42,5 @@
>                                   wrap="true"/>
> +                    <xul:label anonid="event-location"
> +                               crop="end"
> +                               flex="1"
> +                               style="margin: 0;white-space: pre;"

The style for this should likely go into a css class instead. Also, I'm not sure its intended, but since you have flex=1 on both the label and the spacer below, they will take an equal amount of space. You might just want to remove the spacer.

@@ +141,4 @@
>          <setter><![CDATA[
>            this.mOccurrence = val;
>            this.setEditableLabel();
> +          this.setLocationLabel(); //CN - Location - Gorilla Mod

Go ahead and remove the comment here :)

@@ +163,5 @@
>          ]]></body>
>        </method>
> +      <method name="setLocationLabel">
> +      <body><![CDATA[
> +        var evloc = document.getAnonymousElementByAttribute(this, "anonid", "event-location");

Can you use this.eventLocationLabel here?

@@ +170,5 @@
> +        if (showLoc  && (item.getProperty("LOCATION") != null) ){
> +        evloc.textContent = item.getProperty("LOCATION");
> +        }else{
> +        this.eventLocationLabel.setAttribute("hidden", "true" );
> +        }

Could you check the indents and spacing in this function? Spaces instead of tabs, space before/after { and }, space after comma, no space before ); etc.

@@ +272,5 @@
>            this.eventNameLabel.setAttribute("hidden", "true");
>  
>            this.mEditing = true;
> +          var width = document.getAnonymousElementByAttribute(this, "anonid", "eventbox").boxObject.width;
> +          this.eventNameTextbox.setAttribute("style", "width: " + width + "px;");

Can't the width be determined automatically, e.g. with the flex attribute or similar?

::: calendar/base/content/preferences/general.xul
@@ +90,5 @@
>                  </menulist>
> +              </hbox>
> +                <checkbox id="showLocation" pack="end"
> +                          label="&pref.showlocation.label;"
> +                          preference="calendar.view.showLocation"/>

Please check indent here.

::: calendar/base/themes/common/calendar-views.css
@@ +508,5 @@
> +    width: auto;
> +    margin: 0px;    
> +    font-weight: normal;
> +    opacity:0.5;
> +}

You could split this up into two rules, one for .calendar-event-details-core and the other only containing the opacity with .calendar-event-details-core.location-desc.

Also, some trailing whitespaces snuck in here.

::: calendar/locales/en-US/chrome/calendar/preferences/general.dtd
@@ +40,4 @@
>  <!ENTITY pref.accessibility.label "Accessibility" >
>  <!ENTITY pref.systemcolors.label "Optimize colors for accessibility" >
>  <!ENTITY pref.systemcolors.accesskey "c">
> +<!ENTITY pref.showlocation.label "Show Location below Title" >

Please change casing to "Show location below title". I'm never sure how its correct in English, but unless the string is a title itself, most words should be lowercase.

Comment 43

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

Comment on attachment 8567969 [details] [diff] [review]
location_v1.patch

Issues noticed during testing:


1) Create an event in week view via drag
Result: The summary shows up in the middle of the event, instead of at the top
Note: Same thing happens when using click-to-edit

2) I don't know if its just me, but for certain calendar colors (like #7F3A69), when the title text is white (and readable), the location text is not-so-readable. I didn't re-read if it was mentioned before, but could you make sure that at least in system colors mode the normal text color is used, maybe with a slightly increased margin in that case?

3) Probably also discussed in the last comments (didn't read the bug and my battery is about to die), but the preference normally fits better into the "Views" tab, which is already pretty full. Maybe we can cram it on the right side of the "start the week on" option?


r- to get a new patch and fix these two issues, but I think this is a great step forward.

Comment 44

[Roasted]

The bug looks almost solved but the last comment is over a year old and there's no release targeted. What about this bug? Can we still hope?

Comment 45

[Clemens]

Attachment: location_v2.patch

Feedback appreciated, everything works except shorten Text with ... via CSS

Comment 46

[Clemens]

Is been a long time. I went through your list philipp. If you are still alive you can rereview now. and Bring it to it's final place. 
The only thing I didn't find out is how to make the Text (Title, Location) be shortened with CSS text-over-flow.
Currently rebuilding but since these are rather minor changes I do not expect problems.
Cheers

Comment 47

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

Thanks for coming back! I'll take a look at this soon, but may be a while since I'll be at an event next week. You can use crop="end" to make the ... appear. https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/label#a-crop

Comment 48

[Clemens]

I tried to use crop, but it doesn't work. I get line breaks if there is a space in the title or locationname but I don't get a crop like in monthview for example.
<xul:label anonid="event-location" class="calendar-event-details-core location-desc" crop="end"/> doesn't work but I see that there are linebreaks so there is a width. Have not figured that out yet.
As mentioned this does not work for the title as well.

Comment 49

[Clemens]

Hi Philipp, you think we can ship this patch out like that or does it something more?
Kind regards

Comment 50

[[:MakeMyDay]]

Comment on attachment 8815850 [details] [diff] [review]
location_v2.patch

Richard, can you help out Clemens on comment 48?

Comment 51

[Richard Marti (:Paenglab)]

(In reply to [:MakeMyDay] from comment #50)
> Comment on attachment 8815850 [details] [diff] [review]
> location_v2.patch
> 
> Richard, can you help out Clemens on comment 48?

With adding crop="end" at <xul:label anonid="event-location" class="calendar-event-details-core location-desc"/> in calendar-multiday-view.xml
and changing in calendar-view-core.xml
evloc.textContent = item.getProperty("LOCATION");
to
evloc.value = item.getProperty("LOCATION");

gives the ellipsis. Doing the same for the title should also add the ellipsis there.

Comment 52

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

Clemens, can you look into Richard's suggestion?

Comment 53

[Clemens]

I made the changes, it is working (partially) for the location Field and only if there not more Events in the same day and time. The title field is somewhat resistant to cropping maybe that's intended. Uploaded the new version.

Comment 54

[Clemens]

Attachment: location_v3.patch

Comment 55

[moxudo]

Thank you :-)

Comment 56

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

Comment on attachment 8906999 [details] [diff] [review]
location_v3.patch

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

Codewise, some minor nits. Richard, I'd appreciate if you could take a look at the new patch and see how it feels. In the past the cropping in the views has always been a bit annoying, hope we can get it fixed now!

Would also be good to test this with a zero-length event or similar.

::: calendar/base/content/calendar-view-core.xml
@@ +161,5 @@
>        </method>
>  
> +      <method name="setLocationLabel">
> +        <body><![CDATA[
> +          var evloc = document.getAnonymousElementByAttribute(this, "anonid", "event-location");

let's use let instead of var for new functions like this one.

@@ +164,5 @@
> +        <body><![CDATA[
> +          var evloc = document.getAnonymousElementByAttribute(this, "anonid", "event-location");
> +          var item = this.mOccurrence;
> +          var showLoc = Preferences.get("calendar.view.showLocation",false);
> +          if (showLoc  && (item.getProperty("LOCATION") != null)) {

Can you check indent and spacing in this function? There should only be one space before and after operators like &&, the indent should use spaces and blocks should be indented.

@@ +170,5 @@
> +          } else {
> +          this.eventLocationLabel.setAttribute("hidden", "true" );
> +          }
> +        ]]></body>
> +      </method>      

Whitespaces at end of line here

::: calendar/base/content/preferences/views.xul
@@ +70,5 @@
>                          name="calendar.previousweeks.inview"
>                          type="int"/>
> +            <preference id="calendar.view.showLocation"
> +                        name="calendar.view.showLocation"
> +                        type="bool"/> 

Whitespace here too

::: calendar/base/themes/common/calendar-views.css
@@ +500,4 @@
>  }
>  
>  /** End time bar **/
> +.calendar-event-details-core.title-desc,.calendar-event-details-core.location-desc {

Can you put the selectors into multiple lines, or at least put a space after the comma?

::: calendar/locales/en-US/chrome/calendar/preferences/views.dtd
@@ +37,4 @@
>  <!ENTITY pref.numberofweeks.4 "4 weeks">
>  <!ENTITY pref.numberofweeks.5 "5 weeks">
>  <!ENTITY pref.numberofweeks.6 "6 weeks">
> +<!ENTITY pref.showlocation.label "Show location below title" >

I'd just keep this as "Show location" or "Show location in view", the positioning is not really relevant and a technical detail.

Comment 57

[Richard Marti (:Paenglab)]

Comment on attachment 8906999 [details] [diff] [review]
location_v3.patch

The cropping works for the location when the title isn't too long. When in the title there is a word which is longer than the event container the longer text is no more visible and the location has the same width and thus the ellipsis is also no more visible.

This patch isn't the cause of this as it happens without it too. ui-r+

Thank you Clemens.

Comment 58

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

Clemens, can you upload a new patch with nits fixed? Feel free to mark old patches as obsolete when uploading the new version.

Comment 59

[Clemens]

Attachment: locv4.patch

Hi Philipp,
took a while but I cleaned up the patch (whitespaces and indents). Maybe you can rethink the Text 'Show location' as '... with title' or so, because otherwise it might not be clear except for those who already wait 12 years for this feature :)
I hope it still fits the current build.
Cheers,
Clemens

Comment 60

[[:MakeMyDay]]

This patch has string changes (though not sure whether a hidden pref wouldn't be sufficient here), so we would need to get it in in the current cycle to be in the next release. In adding it to the tracking list to keep it on the radar.

Comment 61

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

Attachment: Fix - v5

Here is an updated patch, also with prior review comments taken care of and a few nits fixed

Comment 62

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/c3605c05a4d4
Location does not appear in multiday view event box. r=philipp

Comment 63

[Jorg K (CEST = GMT+2)]

Need target 6.3.

Comment 64

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

Technically this was pushed with late-l10n, since I wasn't able to make it in time for the merge yesterday. Marking as such.

Comment 65

[Jorg K (CEST = GMT+2)]

Beta (TB 60, Calendar 6.2):
https://hg.mozilla.org/releases/comm-beta/rev/9b22a733b765b9a50133661fdccba4e01c3d91de

Comment 66

[moxudo]

Whoop, whoop :-) Thank you all so much!