Last Comment Bug 319909 - Failure to properly serialize/unserialize ics ATTACH properties
: Failure to properly serialize/unserialize ics ATTACH properties
Status: RESOLVED FIXED
: dataloss
Product: Calendar
Classification: Client Software
Component: Internal Components (show other bugs)
: unspecified
: All All
: -- normal (vote)
: 0.9
Assigned To: Fred Jendrzejewski
:
:
Mentors:
http://rfc.net/rfc2445.html#s4.8.1.1
: 382951 (view as bug list)
Depends on:
Blocks: 168680 298358 382951
  Show dependency treegraph
 
Reported: 2005-12-11 15:20 PST by Joey Minta
Modified: 2008-07-14 12:41 PDT (History)
5 users (show)
dbo.moz: wanted‑calendar0.9+
mschroeder: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
testcase (488 bytes, text/plain)
2005-12-11 15:23 PST, Joey Minta
no flags Details
first attempt (15.87 KB, patch)
2005-12-13 19:44 PST, Joey Minta
no flags Details | Diff | Splinter Review
new calIAttachment v2 (16.25 KB, patch)
2005-12-29 07:31 PST, Joey Minta
no flags Details | Diff | Splinter Review
Possible storage support (18.48 KB, patch)
2008-06-30 09:55 PDT, Fred Jendrzejewski
no flags Details | Diff | Splinter Review
calAttach - v3 (19.79 KB, patch)
2008-07-04 08:59 PDT, Fred Jendrzejewski
no flags Details | Diff | Splinter Review
A first possible frontend (52.70 KB, image/png)
2008-07-04 09:02 PDT, Fred Jendrzejewski
chris.j.bugzilla: ui‑review-
Details
tb2 attachment drop down and listbox (11.43 KB, image/png)
2008-07-07 00:09 PDT, Christian Jansen
no flags Details
calAttach - v3 (18.12 KB, patch)
2008-07-07 02:27 PDT, Fred Jendrzejewski
no flags Details | Diff | Splinter Review
The frontend with any needed function (13.66 KB, patch)
2008-07-07 02:32 PDT, Fred Jendrzejewski
no flags Details | Diff | Splinter Review
Mockup for attachments display in dialog (40.04 KB, image/png)
2008-07-07 06:47 PDT, Philipp Kewisch [:Fallen]
no flags Details
calAttach - front endback (29.76 KB, patch)
2008-07-11 13:06 PDT, Fred Jendrzejewski
no flags Details | Diff | Splinter Review
calAttach - v4 (70.26 KB, patch)
2008-07-13 13:44 PDT, Philipp Kewisch [:Fallen]
philipp: review+
chris.j.bugzilla: ui‑review+
Details | Diff | Splinter Review
calAttach - v5 (67.26 KB, patch)
2008-07-14 02:48 PDT, Philipp Kewisch [:Fallen]
philipp: review+
philipp: ui‑review+
Details | Diff | Splinter Review
[checked in] Fix regression - v1 (1.12 KB, patch)
2008-07-14 07:29 PDT, Philipp Kewisch [:Fallen]
philipp: review+
Details | Diff | Splinter Review

Description Joey Minta 2005-12-11 15:20:43 PST
If an vevent or vtodo has an ATTACH property specified, we only keep a small portion of the data around on a round-trip.  Specifically, any parameters that may be specified on the property are lost.

This is probably part of the larger problem that calIAttachment looks rather incomplete.

Testcase to follow.  See also bug 168680.
Comment 1 Joey Minta 2005-12-11 15:23:23 PST
Created attachment 205581 [details]
testcase

The testcase contains 2 attachments, taken directly from examples given in RFC2445.  On a round-trip test in sunbird-20051211, the first attachment is completely lost, since it only contains a parameter.  The second attachment's url remains, but the FMTTYPE is lost.
Comment 2 Joey Minta 2005-12-13 19:44:41 PST
Created attachment 205792 [details] [diff] [review]
first attempt

With this patch, I get no dataloss using the testcase, when editing the event in lightning.  There are substantial changes here to the calIAttachment idl, but that's largely because it was previously unfinished.  We can use bug 168680 to actually expose UI for people to use this feature at some point in the future (or an extension can do it first).  The point is that this gets the backend support for attachments in place.
Comment 3 Michiel van Leeuwen (email: mvl+moz@) 2005-12-15 13:11:33 PST
Comment on attachment 205792 [details] [diff] [review]
first attempt

The interface looks pretty low-level. It would be pretty hard to create the kind off attachments as in the testcase. You need to know all kinds of ical internals to be able to create them. Shouldn't we make the interface be more suited to the task of adding an attachment, instead of trying to reflect the ical standard?
Comment 4 Joey Minta 2005-12-20 09:14:07 PST
(In reply to comment #3)
> (From update of attachment 205792 [details] [diff] [review] [edit])
> The interface looks pretty low-level. It would be pretty hard to create the
> kind off attachments as in the testcase. You need to know all kinds of ical
> internals to be able to create them. Shouldn't we make the interface be more
> suited to the task of adding an attachment, instead of trying to reflect the
> ical standard?
> 

What sort of exposures are you looking for here?  I'm not entirely convinced that we need to really support adding attachments in any form other than adding a sound to an alarm.  Also, the set/getProperty methods are pretty flexible.
Comment 5 Joey Minta 2005-12-20 14:28:26 PST
Comment on attachment 205792 [details] [diff] [review]
first attempt

After chatting about this in IRC, I'm going to take another cut at this.
Comment 6 Joey Minta 2005-12-29 07:31:20 PST
Created attachment 207075 [details] [diff] [review]
new calIAttachment v2

Version 2 includes an easier way to set the format-type of the attachment.  This is the only other part of an ATTACH property that the rfc mentions in any detail.  I couldn't think of anything else to expose that might make this easier to use for creating/editing attachments.  Then again, I'm not entirely sure that I want to see this really exposed to the user at all, but that's another argument...

Just for comparison, this is fairly closely based on calIAttendee.
Comment 7 Michiel van Leeuwen (email: mvl+moz@) 2006-01-01 11:39:33 PST
Comment on attachment 207075 [details] [diff] [review]
new calIAttachment v2

looks ok. r=mvl
Comment 8 Dan Mosedale (:dmose) 2006-01-06 15:50:46 PST
This only supports URI-based attachments, not inline ones.  I've got some ideas on how to implement this, so after discussion with Joey on IRC I'm gonna take this one.
Comment 9 Joey Minta 2006-08-01 13:31:27 PDT
Attachments don't block 0.3.  I think we can do alarms decently well without them.
Comment 10 Matthew (lilmatt) Willis 2006-10-29 19:00:28 PST
Do we want to take Joey's patch as is for now, and add the inline support later?
Comment 11 Dan Mosedale (:dmose) 2007-03-13 14:05:59 PDT
Cleaning up incorrectly assigned bugs; search for dmosecleanup to get rid of this bugmail.
Comment 12 Matthew (lilmatt) Willis 2007-03-13 18:25:37 PDT
jminta:
Your rev2 (unbitrotted) appears to work on ICS calendars, but as evidenced by the "// XXX write me?" comment in calStorageCalendar::writeAttachments, it doesn't work for storage.

Suggestions?  I think having a calAttachments table is a bit overkill for this, but I could be convinced otherwise.
Comment 13 Matthew (lilmatt) Willis 2007-03-13 18:26:37 PDT
So for reference, if you import an ICS file with ATTACHments into a storage calendar, there's dataloss.  We drop them on the floor on the way in.
Comment 14 Fred Jendrzejewski 2008-06-27 04:27:37 PDT
So i started to work on it, because i really miss this feature.	Unfortunately, i'll surely need some help as i am a freshman.

I got the patch in the code, but one part is still missing:

@@ -572,16 +605,20 @@ calItemBase.prototype = {
 
         this.mapPropsToICS(icalcomp, this.icsBasePropMap);
 
         if (this.mOrganizer)
             icalcomp.addProperty(this.mOrganizer.icalProperty);
         for (var i = 0; i < this.mAttendees.length; i++)
             icalcomp.addProperty(this.mAttendees[i].icalProperty);
 
+        for each (att in this.mAttachments) {
+            icalcomp.addProperty(att.icalProperty);
+        }
+
         if (this.mGeneration) {
             var genprop = icalProp("X-MOZILLA-GENERATION");
             genprop.value = String(this.mGeneration);
             icalcomp.addProperty(genprop);
         }
 
         if (this.mRecurrenceInfo) {
             var ritems = this.mRecurrenceInfo.getRecurrenceItems({});


I can't find out where to but it now. The result is a an error, when the createTodo() from calUtils is called.

So if someone is interested in this feature and giving me sometimes some help, i will try my best to get this done.
Comment 15 Daniel Boelzle [:dbo] 2008-06-27 05:05:46 PDT
(In reply to comment #14)
>          for (var i = 0; i < this.mAttendees.length; i++)
>              icalcomp.addProperty(this.mAttendees[i].icalProperty);
> 
> +        for each (att in this.mAttachments) {
> +            icalcomp.addProperty(att.icalProperty);
> +        }
> +

Hi Fred, in general the above looks correct to me (although I haven't had a look at the calIAttachment implementation, how it serializes etc). But please mind that this feature requires enhancing the storage provider to properly read and write multiple attachments, too.

> I can't find out where to but it now. The result is a an error, when the
> createTodo() from calUtils is called.
I don't understand that. Could you please explain in more detail?
Comment 16 Fred Jendrzejewski 2008-06-27 07:02:44 PDT
> > I can't find out where to but it now. The result is a an error, when the
> >  createTodo() from calUtils is called.
> I don't understand that. Could you please explain in more detail?

The patch is rejected. So i have to merge it in the code by hand. I can do this for the hole patch, except the part of the patch (calAttachment v2) that i posted. I can't find out where to put this part.

The result of the changes that i did until now is an error, when the function "createTodo" is called and the Tasks aren't loaded anymore.

I think, the first thing before the storage problem will be to merge in the old patch.

I hope that i explained it better now.
Comment 17 Philipp Kewisch [:Fallen] 2008-06-27 07:16:04 PDT
+        for each (att in this.mAttachments) {
+            icalcomp.addProperty(att.icalProperty);
+        }
+

Looks like this needs to go in 
http://lxr.mozilla.org/mozilla1.8/source/calendar/base/src/calItemBase.js#711
Comment 18 Fred Jendrzejewski 2008-06-30 09:55:41 PDT
Created attachment 327421 [details] [diff] [review]
Possible storage support

I implemented storage support. I am posting this only for reasons that my work can be somehow observed. I didn't test it very much (not at all), but Sunbird runs now without making any displayed errors. The things i'd like to do for testing is:
1.) Change the front-end to work with this implementation.
2.) Look for an intelligent way to work with base64 strings and how to export files.
Comment 19 Fred Jendrzejewski 2008-07-02 03:04:50 PDT
Hello, some problems that i can't interpret until now:
1.)When i want to use the command 
"attachment.uri = "
i get the following error message:

reserved slot index out of range

So my storage isn't working ? 
2.)The other thing that doesn't work is
atts = item.getAttachments({});
for each ( att in atts){...}

att is always null.
3.) I can normally start sunbird and create a new profile. But i don't get any information about database updates and i have never seen one.
Comment 20 Fred Jendrzejewski 2008-07-04 08:59:58 PDT
Created attachment 328138 [details] [diff] [review]
calAttach - v3

The calAttachment is storaged now. I tested the interface on the possibility to delete and save attachments for any item extensivly. I implemented the encoding property as this one is important for the export and the import. I tested neighter import nor export at the moment. I need some opinions about the new class and about the export/import.
1.) It has absolutly no sense to export the uri of a local file. I also think that the "ATTACH" property is used for local files and the "URL" property for distant files.
So i think that any attachments should be exported as a base64 uri.
2.) What should be done this base64 imported uris ? Should we suppose to save them locally during the import?
3.) How do we open attachments? Should we open it with a dialog that is similar to the Thunderbird dialog.

Bye,
Fred
Comment 21 Fred Jendrzejewski 2008-07-04 09:02:01 PDT
Created attachment 328140 [details]
A first possible frontend

It is possible to
1.) Attach files
2.) Delete them
Comment 22 Fred Jendrzejewski 2008-07-05 04:53:06 PDT
So export seems to work, but local uri aren't transformed.
When i want to import a calendar it never works, because he can't handle line 544 of the patch:
writeAttachments(item, olditem){
...
ap.uri = att.uri.spec;

...}
He never gets any data into the uri. And i just can't find the reason for it.
Comment 23 Philipp Kewisch [:Fallen] 2008-07-05 12:13:32 PDT
(In reply to comment #20)
> Created an attachment (id=328138) [details]
> calAttach - v3
> 
> The calAttachment is storaged now. I tested the interface on the possibility to
> delete and save attachments for any item extensivly. I implemented the encoding
> property as this one is important for the export and the import. I tested
> neighter import nor export at the moment. I need some opinions about the new
> class and about the export/import.
> 1.) It has absolutly no sense to export the uri of a local file. I also think
> that the "ATTACH" property is used for local files and the "URL" property for
> distant files.
The URL property is not used for remote files. We do this now, but it is not supposed to. the URL property should be used if there is an alternative representation of the event (i.e a html page).

To attach a remote url to an event, the ATTACH property needs to be used. For local urls, the file could be attached base64 encoded as you suggested and I think this is the way to go. I'm eager to hear Daniel's opinion on this though.

If we stay with the terms "Link" for linked remote urls and "File" for local urls, then I don't think we need additional informative UI. We should make sure that if the user adds a link with a file:// url, then the file should be attached in binary form.



> So i think that any attachments should be exported as a base64 uri.
> 2.) What should be done this base64 imported uris ? Should we suppose to save
> them locally during the import?
This should be provider dependant. The ics provider will store the attachment as is, the storage provider can use a BLOB (see http://www.sqlite.org/datatype3.html) to save binary attachments.

Other providers like gdata don't support attachments. There should be a calendar property, (i.e calendar.getProperty("capabilities.attachments.supported") !== false) that disables options to add attachments in the dialog.



> 3.)  How do we open attachments? Should we open it with a dialog that is similar
> to the Thunderbird dialog.
Yes, I think a such dialog should be used. Similar UI as in thunderbird should also be used to show which attachments have been added to the event.

Serialization of the attachment should be done similar to how thunderbird acts when an attachment is added. I'm not sure how it actually is, but for example if the user attaches a file in the event dialog, then changes the attached file locally, and then saves the event, the attachment should also be changed.

I was initially thinking it would be great to store just the url when running and just serialize when writing the attachments, but after some thinking I came to the conclusion this is unintuitive.
Comment 24 Fred Jendrzejewski 2008-07-05 13:24:43 PDT
> The URL property is not used for remote files. We do this now, but it is not
> supposed to. the URL property should be used if there is an alternative
> representation of the event (i.e a html page).
So this has to be represented in an intuitive way in the UI, if we really want this feature.
> Other providers like gdata don't support attachments. There should be a
> calendar property, (i.e
> calendar.getProperty("capabilities.attachments.supported") !== false) that
> disables options to add attachments in the dialog.
This is theoretically already built in.
>Yes, I think a such dialog should be used. Similar UI as in thunderbird should
>also be used to show which attachments have been added to the event.
I used the thundbird code as an example for the hole file-attaching, so it is already very similiar.
> I was initially thinking it would be great to store just the url when running
> and just serialize when writing the attachments, but after some thinking I came
>to the conclusion this is unintuitive.
The advantages of only storing the local url are the followings:
- always the actual version
- no copies of the file on the same computer, this would be a waste of resources.(always getting the newest version, storing the copy, load it from the db)

And i don't see how to do it as well with a base64 stored file.
Comment 25 Christian Jansen 2008-07-07 00:07:23 PDT
Comment on attachment 328140 [details]
A first possible frontend

We should use the same wording and UI as Thunderbird does.

For the menu it would be:

Attach
  Files(s)...
  Web Pages

Attachment List Box:

IMO the File URLs are a not needed. Can't we show the file, only?

In case of an URL we should continue to show the whole URL.

We should also offer an Context menu for:

Open,Delete, Select All
See attachment.


Overall I think this UI needs some fine tuning.
Comment 26 Christian Jansen 2008-07-07 00:09:46 PDT
Created attachment 328304 [details]
tb2 attachment drop down and listbox
Comment 27 Fred Jendrzejewski 2008-07-07 02:27:14 PDT
Created attachment 328307 [details] [diff] [review]
calAttach - v3

We can store attachments, delete them, import and export them. But there is still a very big problem.
We get handle the VALUE=BINARY property, so we can't handle properly base64 url's and this is the only really interesting point in the hole work. It has to be done in the libical and i very very far from calling me a good c-programmer.
Comment 28 Fred Jendrzejewski 2008-07-07 02:32:39 PDT
Created attachment 328308 [details] [diff] [review]
The frontend with any needed function

The following things are possible:
- attach a file
- attach a url
- delete the attachments with a context menu or the keyboard
- attach a file by clicking the listbox
- open the file by doubleclicking the item

Problems:
-resizing of the box after adding an attachment
- should the box always be visible? at the moment it is only visible after adding the first attachment, this surely has to changed
- what are reasonable commands for the popup menu? I think delete and open are sufficient
- do we need the possibility to delete all attachments at once?
Comment 29 Philipp Kewisch [:Fallen] 2008-07-07 06:47:49 PDT
Created attachment 328319 [details]
Mockup for attachments display in dialog

With the frontend patch applied, I get a listbox that has no border which extends past the dialog (probably one too many flex attributes), with the whole filename as an url.

Also, I think we should make it look more like the tb attachments dialog (display an icon, more than one column). I have attached a mockup (just for the attachments part, not for the toolbar) of how I picture it. Any comments welcome!  To stay consistant, I think an icon should also be displayed for URLs.

...

I just took a closer look at the code and saw that you are actually setting an image for the link. This doesn't seem to work out for me (linux), no image is displayed. Generally I think you are on the right track!


(Fred btw, for your future patches, please put all files into one instead of splitting up :-)
Comment 30 Fred Jendrzejewski 2008-07-07 09:07:26 PDT
> Also, I think we should make it look more like the tb attachments dialog
> (display an icon, more than one column). I have attached a mockup (just for the
> attachments part, not for the toolbar) of how I picture it. Any comments
> welcome!  To stay consistant, I think an icon should also be displayed for
> URLs.
Looks good with two attachments. I'd suggest the following, if it is possible to implement it:
- one row is always displayed if there is an attachment or not, so it is possible to use maybe a drag and drop/to doubleclick/to use the context menu/to no that it is there
- 2 attachments per row with 3 rows max. then it should become scrollable. Maybe three rows from the beginning are fine too.
- only the name of the file is displayed, if needed it is cropped and the full name is part of the tooltip - same for urls

About the backend:
Does anyone have a good example how to proceed to implement the VALUE=BINARY property ? Would be great if someone could do the hole part, but i could try if i had an example.
Comment 31 Fred Jendrzejewski 2008-07-09 14:21:24 PDT
hello, so here is my proposition how to handle import of base64 strings and export of locale files:
Import : The import of the attachment shouldn't doesn't fail silently, but an errormessage should be raised.
Export: It would be optimal to have some kind of tags; So we'd add the filenames as tags. Actually this doesn't seems to be possible. I am not sure if we could use the relationship property maybe ?
I'll try to get done all the strings until monday, this should be possible.
Comment 32 Fred Jendrzejewski 2008-07-11 13:06:12 PDT
Created attachment 329112 [details] [diff] [review]
calAttach - front endback

This patch is almost everythin I can do. The issues are the followings:

Frontend:
- moz-icon doesn't work -> no icons
- I have to remove the listboxbody statement from the css and I can't get the class to be applied for the attendee listboxes
- the formattype of an attachment isn't set
- I don't use "Select All" in the popup because it seems not really logic why I should introduce this, if I only need delete all

Backend:
- What do we do with binary files ?

My suggestion :
The frontend and the backend works well for webpages. So this can be used IMHO. And we have to find an intelligent solution how we want to handle binary content laterly. If we want to support it only partly, or later or full.

I hope this hole bunch of work will be useful one day,
Fred
Comment 33 Philipp Kewisch [:Fallen] 2008-07-13 13:44:09 PDT
Created attachment 329346 [details] [diff] [review]
calAttach - v4

This patch is almost everythin I can do. The issues are the followings:

Frontend:
- moz-icon doesn't work -> no icons
Works, just needs some more libs installed.

- I have to remove the listboxbody statement from the css and I can't get the
class to be applied for the attendee listboxes
Fixed

- the formattype of an attachment isn't set
Didn't fix this since its not really important for URLs.

- I don't use "Select All" in the popup because it seems not really logic why I
should introduce this, if I only need delete all
I agree.

Backend:
- What do we do with binary files ?
As discussed on irc, we should disable/hide those options. I chose to disable since it doesn't make sense to have just one menuitem. Is this ok from a UI perspective? Of course its strange to have a disabled "File(s)..." menuitem, but
 I think we can get away with it.

I'm going to check this in (given review) tomorrow in the course of the day, before the string freeze.
Comment 34 Fred Jendrzejewski 2008-07-13 14:52:26 PDT
Some Questions:
- A function can have an attributes like mValue ? This is nice.
- The moz-icon can be set to attachment.uri.spec or am I overlooking something ?
- a planned with christian that the rows for the attachments are open if the attachments are enabled, so a drag and drop can be implemented one day and it is possible to use the context menu. So the last line of updateAttachment and its call in addAttachment isn't needed

But as those are UI questions I can't see any problem and I am interested in Christians opinion. I don't know why but I am not authorized to edit the attachment, but my r+ is sure :)
Comment 35 Philipp Kewisch [:Fallen] 2008-07-14 01:59:02 PDT
Comment on attachment 329346 [details] [diff] [review]
calAttach - v4

(In reply to comment #34)
> Some Questions:
> - A function can have an attributes like mValue ? This is nice.
Yes, a function is an object too, so you can assign attributes freely.


> - The moz-icon can be set to attachment.uri.spec or am I overlooking something
I didn't really look into how moz-icon works, I'll do so and then probably pass the uri spec.

> ?
> - a planned with christian that the rows for the attachments are open if the
> attachments are enabled, so a drag and drop can be implemented one day and it
> is possible to use the context menu. So the last line of updateAttachment and
> its call in addAttachment isn't needed
I was planning to mimic thunderbird here. The attachments listbox is initially closed and only shown when there are attachments. The only thing I've done differently is that the listbox is hidden when the last attachment is removed. Thunderbird leaves the listbox there.

With DND I think we can get away with just showing the listbox after a certain timeout when the user drags a file over the dialog, and allowing to drop the file on the whole dialog instead of only on the listbox.

r=fred based on previous comment.
Comment 36 Christian Jansen 2008-07-14 02:02:43 PDT
Comment on attachment 329346 [details] [diff] [review]
calAttach - v4

Hi,
some feedback after a short UI-Review (on Mac)

- Attached URLs provides always an / at to the end. This should be removed.

- Would it be possible to offer a two column layout (like suggested in
https://bugzilla.mozilla.org/attachment.cgi?id=328319)?

- It would be good if one could resize the attachment box hight by adding a
splitter between Details and attachment.

- The context menu is ok, IMO Attach File(s) should be hidden if the Provider
can't handle attachments.

But these things can be fixed in a spin-off bug. ui=christian
Comment 37 Philipp Kewisch [:Fallen] 2008-07-14 02:48:49 PDT
Created attachment 329404 [details] [diff] [review]
calAttach - v5

(In reply to comment #36)
> (From update of attachment 329346 [details] [diff] [review])
> Hi,
> some feedback after a short UI-Review (on Mac)
> 
> - Attached URLs provides always an / at to the end. This should be removed.
Done

> 
> - Would it be possible to offer a two column layout (like suggested in
> https://bugzilla.mozilla.org/attachment.cgi?id=328319)?
Not that easy, I think we would need to create two listboxes that have no margin/padding/border between each other and then add the items into the two listboxes to make it look like one listbox. -> Followup bug

> 
> - It would be good if one could resize the attachment box hight by adding a
> splitter between Details and attachment.
I misunderstood this a bit on the phone, for now I added a separator between the two, making it a splitter is something also for a followup.

> 
> - The context menu is ok, IMO Attach File(s) should be hidden if the Provider
> can't handle attachments.
Removed attach files from the UI since no provider supports this at the moment.
Comment 38 Philipp Kewisch [:Fallen] 2008-07-14 05:56:26 PDT
Checked in on HEAD and MOZILLA_1_8_BRANCH

-> FIXED
Comment 39 Philipp Kewisch [:Fallen] 2008-07-14 05:58:14 PDT
*** Bug 382951 has been marked as a duplicate of this bug. ***
Comment 40 Philipp Kewisch [:Fallen] 2008-07-14 07:29:17 PDT
Created attachment 329447 [details] [diff] [review]
[checked in] Fix regression - v1

We forgot to change an entity label at one point, this fixes.

r=dbo as discussed.

Note You need to log in before you can comment on or make changes to this bug.