351168 - Build Lightning+WCAP when building nightlies

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Matthew (lilmatt) Willis]

We want to build a version of Lightning with WCAP enabled when we build nightlies.
WCAP is off by default. To prevent us from having two actual Lightning's in circulation claiming to be 0.3, we should create a small "WCAP Enabler" extension, and then package both the "stock" Lightning with the "WCAP Enabler" in a multi-extension XPI.

Yet to be determined is distribution. We're not sure if this wants to be hosted from addons.mozilla.org or not.

Comment 1

[Matthew (lilmatt) Willis]

Attachment: Creates wcapLightning multi-extension XPI and wcapEnabler extension

This was my first cut. I believe [e-mail] has updates for it.

Comment 2

[Matthew (lilmatt) Willis]

Requesting blocking0.3? per discussions with dmose and mozilla.

Comment 3

[Daniel Boelzle [:dbo]]

Attachment: lightning-wcap.xpi = { lightning.xpi, wcap-enabler.xpi }

Thanks Matt for the patch. As previuosly agreed, I have made some changes to it:
- naming to non-caml: i.e. wcap-enabler, lightning-wcap
- added uuids
- fixed build dependency to lightning.xpi
- split up tinderbox patch

Comment 4

[Daniel Boelzle [:dbo]]

Attachment: pushing lightning-wcap.xpi to xpi-stage

Comment 5

[J. Paul Reed [:preed]]

Comment on attachment 236702 [details] [diff] [review]
pushing lightning-wcap.xpi to xpi-stage

I don't know all the history/details, but if the -wcap.xpi is  optional/not part of the regular lightning release or you could build the -wcap XPI by itself, you probably want this to be its own if(-e $lightningWcapXpi) branch, instead of including it in this branch.

Alternatively, you could do if ((-e $lightningXpi || -e $wcapXpi) && ...), but then you have to disambiguate in the block anyway so the cp's don't fail.

I'll r- to clear this out of my queue, but if I have some of the above details wrong, LMK, and I'll look at this again.

Comment 6

[Matthew (lilmatt) Willis]

Attachment: tinderbox patch - rev1 - Addresses preed's comments

Returns busted if things just don't work out the way we planned

Comment 7

[J. Paul Reed [:preed]]

Comment on attachment 236843 [details] [diff] [review]
tinderbox patch - rev1 - Addresses preed's comments

Much better. r=preed.

Comment 8

[Matthew (lilmatt) Willis]

Comment on attachment 236843 [details] [diff] [review]
tinderbox patch - rev1 - Addresses preed's comments

BIG OL' DISCLAIMER:
-------------------

Don't check this patch in until the OTHER one (with the Makefiles and stuff) is reviewed and checked in.  Otherwise we'll stop having Lightning builds.

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

According to my notes from our phone meeting, ssa suggested leaving the new event dialog preffed off for 0.3, in order to allow for apples-to-apples analysis of feedback from WCAP users about how the default dialog works for them.

Also, do we want to make the WCAP enable extension have an extension dependency on Lightning?

Comment 10

[Matthew (lilmatt) Willis]

Comment on attachment 236701 [details] [diff] [review]
lightning-wcap.xpi = { lightning.xpi, wcap-enabler.xpi }

Before dmose takes a whack at this, I've got some nits...


>diff -x CVS -a -U 8 -pN -r /cygdrive/d/pim/mozilla/mozilla_ref/calendar/prototypes/wcap/lightning-wcap/install.rdf mozilla/calendar/prototypes/wcap/lightning-wcap/install.rdf
>+        <em:id>{2C3512D6-F84B-4b20-AF47-5AB0F0B6192E}</em:id>
We should be using human-readable UIDs for new apps and extensions (as long as they don't have to work in Firefox/Tbird 1.0.x)
I suggest [e-mail] 


>diff -x CVS -a -U 8 -pN -r /cygdrive/d/pim/mozilla/mozilla_ref/calendar/prototypes/wcap/wcap-enabler/Makefile.in mozilla/calendar/prototypes/wcap/wcap-enabler/Makefile.in
>+PREF_JS_EXPORTS = $(srcdir)/wcapEnabler.js
If you're changing from CamelHumps to hyphens, let's do it here as well (and rename the file)


>diff -x CVS -a -U 8 -pN -r /cygdrive/d/pim/mozilla/mozilla_ref/calendar/prototypes/wcap/wcap-enabler/install.rdf mozilla/calendar/prototypes/wcap/wcap-enabler/install.rdf
>+        <em:id>{E738F0DA-ED22-4baa-A607-EDE6A66D7F20}</em:id>
Again, let's use a human-readable UID.
[e-mail]?


>diff -x CVS -a -U 8 -pN -r /cygdrive/d/pim/mozilla/mozilla_ref/calendar/prototypes/wcap/wcap-enabler/wcapEnabler.js mozilla/calendar/prototypes/wcap/wcap-enabler/wcapEnabler.js
First off, let's rename the file to use hyphens as I mentioned earlier.

>+// This file contains the preferences to enable WCAP support in Lightning
>+pref("calendar.prototypes.wcap", true);
>+pref("calendar.wcap.enabled", true);
We should probably add a comment to each of these lines describing what effect they each have.
In addition, I should have set "calendar.prototypes.wcap" to false so we didn't turn on the prototype event dialog.


>Index: mozilla/extensions/lightning/Makefile.in
>===================================================================
>-DIRS 		= ../../calendar/lightning
>+DIRS = ../../calendar/lightning ../../calendar/prototypes/wcap/lightning-wcap
Let's split this up in like this:

DIRS = \
    $(topsrcdir)/calendar/lightning \
    $(topsrcdir)/calendar/prototypes/wcap/lightning-wcap \
    $(NULL)

Note my lack of tabs. That's on purpose :)

Comment 11

[Matthew (lilmatt) Willis]

(In reply to comment #9)
> Also, do we want to make the WCAP enable extension have an extension dependency
> on Lightning?

I misspoke in IRC. Basic "don't install A unless B is already installed" dependency does work at the moment (see bug 298497) but it's extremely rudimentary, having to support to _help_ you find the missing dependency. (Sounds like .rpm a lot of the time) :)

I don't think adding that dependency checking to wcap-enabler gains us more than the grief it might well cause. I (with no real data to base this on, mind you) forsee wcap-enabler attempting to get installed first, so that even though both xpis are in the multi-xpi, wcap-enabler refuses to install since lightning is not installed.  On the other hand, if I install wcap-enabler without lightning, nothing in my life really changes.  I have two prefs set that without lightning, nothing relies on.


Another nit on the patch that needs resolution is hosting and updateURLs. Will we host these through a.m.o?  We want updateURL elements regardless, and if we're not using a.m.o, we'll need to think of a good place for the update.rdfs to live.

Comment 12

[Matthew (lilmatt) Willis]

(In reply to comment #11)
> ...having to support to _help_ you find the missing dependency...
That should be having _no_ support...

Comment 13

[Daniel Boelzle [:dbo]]

(In reply to comment #9)
> According to my notes from our phone meeting, ssa suggested leaving the new
> event dialog preffed off for 0.3, in order to allow for apples-to-apples
> analysis of feedback from WCAP users about how the default dialog works for
> them.

Right, I remember. Though I think the prototype event dialog fits WCAP users' needs better, mainly because it supports accepting invitations(!), auto-completion from LDAP and freebusy information. Our QA team has extensively tested it with the WCAP provider. Thus with respect to usability and quality, this is IMO the better default combination for users.

Comment 14

[Daniel Boelzle [:dbo]]

(In reply to comment #10)
> (From update of attachment 236701 [details] [diff] [review] [edit])
> We should be using human-readable UIDs for new apps and extensions (as long as
> they don't have to work in Firefox/Tbird 1.0.x)

Are human-readable ids generally preferred over uuids nowadays in mozilla?

Comment 15

[Matthew (lilmatt) Willis]

(In reply to comment #14)
> Are human-readable ids generally preferred over uuids nowadays in mozilla?

For extensions, yes.
See http://developer.mozilla.org/en/docs/install.rdf#id

Obviously not for nsIModules and other internal stuff

Comment 16

[Daniel Boelzle [:dbo]]

Attachment: updated lightning-wcap.xpi = { lightning.xpi, wcap-enabler.xpi }

Comment 17

[Matthew (lilmatt) Willis]

Comment on attachment 238208 [details] [diff] [review]
updated lightning-wcap.xpi = { lightning.xpi, wcap-enabler.xpi }

This looks good. Thanks for the changes.

I want to hold off on giving review until we decide on that "todo" pref, and until we decide what em:updateUrl to add to these.

I don't want these extensions in the wild without the updateUrl

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #13)
> Thus with respect to usability and quality, this is IMO the better default
> combination for users.

The interesting thing about making the regular dialog the default and release-noting the pref is that since most users don't closely read the release notes, they're likely to try the regular dialog first.  When they realize that features they care about are missing, they'll be motivated to figure out how to access those features: flipping the pref and using the tabbed dialog.  It should then be possible to get at least some level of feedback from them about the usability of the simplified dialog vs. the tabbed dialog.  If we ship with the tabbed dialog set by default, they'd never be motivated to actually try the simplified version and be in a position to give that feedback.

Comment 19

[Michael Büttner (no reviews TFN)]

Although I can follow the previous reasoning, wouldn't it be a good idea to mention the different dialogs on amo and/or on the website/wiki in order to collect user feedback? I suspect that only a tiny fraction of users read the release notes and know how to toggle the pref. Chances are better that users try both versions and compare without the need to manually configure prefs.

Comment 20

[Matthew (lilmatt) Willis]

Attachment: updated version of daniel's patch

This is the same as daniel's patch, only with added updateURLs and setting the prototype event dialog pref to false, which are the nits I wanted addressed.

I applied them and am reattaching this because daniel's on holiday.

r1=lilmatt

Comment 21

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #19)
> Although I can follow the previous reasoning, wouldn't it be a good idea to
> mention the different dialogs on amo and/or on the website/wiki in order to
> collect user feedback? I suspect that only a tiny fraction of users read the
> release notes and

Making this info more widely available sounds like a good idea, but I'm not sure that that's likely to make a tremendous difference in discoverability, unless people read those other places more than they tend to read release notes.

> know how to toggle the pref. Chances are better that users
> try both versions and compare without the need to manually configure prefs.

I assume you mean by enabling/disabling the WCAP-enabler extension in the extensions dialog?  This still seems to have the problem that none of the WCAP users are likely to be motivated to try the simpler dialog once they've used the more advanced one, since the "feature-completeness" is going to outweigh any "simplicity/ease-of-use" wins, especially since any users who think to use the enable/disable UI for this seem likely to be power-users anyway.  Do you disagree?

Comment 22

[Dan Mosedale (:dmosedale, :dmose)]

Using that UI would also turn off WCAP support itself, which makes it seem even less desirable for that set of users.

Comment 23

[Daniel Boelzle [:dbo]]

(In reply to comment #21)
> extensions dialog?  This still seems to have the problem that none of the WCAP
> users are likely to be motivated to try the simpler dialog once they've used
> the more advanced one, since the "feature-completeness" is going to outweigh
> any "simplicity/ease-of-use" wins, especially since any users who think to use
> the enable/disable UI for this seem likely to be power-users anyway.  Do you

Dan, I see your point. But I assume WCAP users will most likely miss a feature to accept an invitation. The other features the prototype UI offers are nice to have but essentially redundant (i.e. freebusy information, LDAP auto completion, different tabbed design).
Concerning invitations, AFAIK even ITIP is no solution. It usually works for calendar-systems interoperation only: The Sun server only sends an ITIP request if the attendee has no account, so does google cal (IIRC).
This IMO a valid/needed feature of an event dialog.

Comment 24

[Michael Büttner (no reviews TFN)]

(In reply to comment #21)
> I assume you mean by enabling/disabling the WCAP-enabler extension in the
> extensions dialog?
no, i was refering to toggle the standard/advanced dialog pref.

> This still seems to have the problem that none of the WCAP
> users are likely to be motivated to try the simpler dialog once they've used
> the more advanced one, since the "feature-completeness" is going to outweigh
> any "simplicity/ease-of-use" wins, especially since any users who think to use
> the enable/disable UI for this seem likely to be power-users anyway.  Do you
> disagree?
i personally don't agree that both dialogs are so much different at all. the socalled advanced dialog has not a tremendous amount of additional features, it all boils down to 1) ldap support 2) accept/decline invitations 3) support for freebusy. the real difference between the dialogs is the layout and structure. and this is exactly where i would like to get feedback about (without forcing users to discover hidden preferences). i really wouldn't mind if this feedback results in people being totally happy with the standard dialog, by the way. but besides that, if it doesn't hurt, why can't we just push forward and give the 'advanced' dialog a chance?

Comment 25

[Daniel Boelzle [:dbo]]

> I applied them and am reattaching this because daniel's on holiday.
Matt agreed to take over the bug.
IMO there aren't any leftovers except for the ongoing prototypes UI pref discussion. We should split that discussion off the overall issue and bring in this patch soon to have the chance to discover further problems in daily tinderbox life. Changing the pref later on (depending on the discussion's outcome) is no big issue.

Comment 26

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 238524 [details] [diff] [review]
updated version of daniel's patch

If you would prefer to have the non-standard dialog preffed on by default, I can live with that.

In general, looks good; r=dmose.  In order to avoid taking folks by surprise, let's not land this quite yet, I'd like to get some feedback from the upcoming newsgroup post.  Once that happens, I'll mark r+ on the patch.

Comment 27

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 238524 [details] [diff] [review]
updated version of daniel's patch

The announcement has been around in the newsgroup since late last week.  Feel free to land at will.

Comment 28

[Matthew (lilmatt) Willis]

Comment on attachment 238524 [details] [diff] [review]
updated version of daniel's patch

Patch landed on MOZILLA_1_8_BRANCH and trunk.

Comment 29

[Matthew (lilmatt) Willis]

Comment on attachment 236843 [details] [diff] [review]
tinderbox patch - rev1 - Addresses preed's comments

Patch checked in on trunk. (No /m/tools/tinderbox dir on branch)

Comment 30

[Matthew (lilmatt) Willis]

-> FIXED