984917 - Remove Sunbird code Redux

Status: RESOLVED FIXED

(Calendar :: Sunbird Only, defect)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v1

Builds and passes mozmill and xpcshell locally.

Comment 1

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

Comment on attachment 8392881 [details] [diff] [review]
patch, v1

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

r=philipp with these changes:

::: calendar/base/content/calendar-chrome-startup.js
@@ +16,5 @@
>      // Load the Calendar Manager
>      loadCalendarManager();
>  
>      // Restore the last shown calendar view
> +    ltnSwitchCalendarView(getLastCalendarView(), false);

Maybe you can rename ltnSwitchCalendarView to switchCalendarView, now that we don't have two functions anymore?

::: calendar/base/content/calendar-common-sets.js
@@ +237,5 @@
>                          }
>                      }
>  
>                      // If calendar is not in foreground, let the default controller take
> +                    // care. If we don't have a default controller, just continue.

So in theory we will always have a default controller. I don't see code that checks for the default controller, can you either just remove that comment or remove the code that special cases if there is no controller?

Same with other places where a similar comment shows up.

@@ +429,5 @@
>      onEvent: function cC_onEvent(aEvent) {
>      },
>  
>      isCalendarInForeground: function cC_isCalendarInForeground() {
> +        return gCurrentMode && gCurrentMode != "mail";

We should get rid of this mode switching chaos, but in a followup bug. Can you file?

@@ +743,5 @@
>      },
>  
>      doCommand: function doCommand(aCommand) {
>          switch (aCommand) {
> +            // These commands are overridden.

I think you can just remove this comment

@@ +818,5 @@
>      }
>  
>      // This needs to be done for all applications
>      top.controllers.insertControllerAt(0, calendarController);
>      document.commandDispatcher.updateCommands("calendar_commands");

Please put the last lines (without comment) into the if/else block and switch the if/else block to avoid the extra inversion of tbController. This way we also remove the strange control flow with that extra return statement.

::: calendar/locales/en-US/README.txt
@@ +1,1 @@
> +For information about installing, running and configuring Lightning 

While you are here could you remove the whitespaces?

::: calendar/locales/en-US/chrome/calendar/calendar.properties
@@ +336,5 @@
>  
>  # Master Password
>  changeMasterPassword=Change Master Password…
>  pw_change2empty_in_fips_mode=You are currently in FIPS mode. FIPS requires a non-empty Master Password.
>  pw_change_failed_title=Password Change Failed

Looks like there are some more obsolete Sunbird strings here related to the master password. If you like, check if thats the case.

::: calendar/locales/l10n.ini
@@ +10,4 @@
>  #
>  # [extras]
>  #
>  # [include_toolkit]

Do we still need the [include_toolkit] section?

::: calendar/providers/gdata/install.rdf
@@ +37,5 @@
>          <em:id>{e2fda1a4-762b-4020-b5ad-a41df1933103}</em:id>
>          <em:minVersion>1.9a1</em:minVersion>
>          <em:maxVersion>@CALENDAR_VERSION@</em:maxVersion>
>        </Description>
>      </em:requires>

While you are here go ahead and remove the whole <em:requires> block, last I heard its no longer supported anyway.

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch, v2, r=Fallen

> ::: calendar/base/content/calendar-common-sets.js
> @@ +429,5 @@
> >      onEvent: function cC_onEvent(aEvent) {
> >      },
> >  
> >      isCalendarInForeground: function cC_isCalendarInForeground() {
> > +        return gCurrentMode && gCurrentMode != "mail";
> 
> We should get rid of this mode switching chaos, but in a followup bug. Can
> you file?
Filed bug 986666.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/53d94b6aec5e