1063487 - (Oauth) Authentication is broken in Calendar

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Calendar, defect)

Description

[Chris Lord [:cwiiis]]

If you try to log into a Google Calendar account in the calendar app, you'll be presented with a blank window that you can't escape from.

It seems the html for the oath window was updated without updating the corresponding JavaScript (how did this get past review?)

Comment 1

[Chris Lord [:cwiiis]]

Attachment: Update oauth JS to correspond to html

Comment 2

[Miller Medeiros [:millermedeiros]]

[Blocking Requested - why for this release]: regression! very important feature! user won't be able to add a Google calendar.

Comment 3

[Miller Medeiros [:millermedeiros]]

Comment on attachment 8484926 [details] [review]
Update oauth JS to correspond to html

LGTM. changes are minimal, tested on the device. Was able to add a Google Calendar and also click the "back" button.

Comment 4

[Chris Lord [:cwiiis]]

Merged: https://github.com/mozilla-b2g/gaia/commit/4f879c347b14f76e6717d57a760de464432ce2ba

Comment 5

[Gareth Aye [:gaye] (back from PTO)]

Is there any way we can prevent future regressions of this sort?

Comment 6

[Miller Medeiros [:millermedeiros]]

(In reply to Gareth Aye [:gaye] from comment #5)
> Is there any way we can prevent future regressions of this sort?

manually test the add account when a patch touches any code related to the oauth window :P

this was only introduced because the patch affected multiple views at once and I forgot to add a google account while testing it - I manually tested other views to check the design but didn't realize the oauth window also had changes (code changes looked OK, you can see I added comments related to the layout of other views...).

this is the kind of bug that is easy to spot and that we definitely wouldn't ship with it, the likelihood of introducing this kind of regression is minimal, and the fix is usually trivial - if you are coding something related to the oauth window you will probably test it on the phoned during development..

Comment 7

[Chris Lord [:cwiiis]]

I think we could restructure the unit tests/app a bit so that they inject less of their own markup - that would have caught this (i.e. if it was testing against the markup that's actually in the app, rather than a copy of it in the test). That's a non-trivial change, though.

A marionette test for the oauth account flow would be ideal, but again, is non-trivial.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Please request Gaia v2.1 approval on this patch when you get a chance :).

Comment 11

[Chris Lord [:cwiiis]]

Comment on attachment 8484926 [details] [review]
Update oauth JS to correspond to html

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 
[User impact] if declined: Can't add or re-log-in oauth based Calendar accounts (such as Google), and have to close the Calendar app if you try (leaves it in an unrecoverable state)
[Testing completed]: Locally and been on master for a few days now
[Risk to taking this patch] (and alternatives if risky): Low risk vs. not taking it
[String changes made]: None

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Needs rebasing for v2.1 uplift.

Comment 13

[Chris Lord [:cwiiis]]

Attachment: Backport to 2.1

Comment 14

[Chris Lord [:cwiiis]]

I'll merge the rebase when tests are green.

Comment 15

[Chris Lord [:cwiiis]]

Something is up with Gb, but doesn't appear to be anything to do with this (backed up by kgrandon on IRC), so merged: https://github.com/mozilla-b2g/gaia/commit/7627105347fe8e8755002c414c124c9bd87691ec

Comment 16

[Edward Chen[:edchen]]

[Environment]
Gaia      944e5b4582c4efa1b67cd33245dbb8f6aa25d09f
Gecko     https://hg.mozilla.org/releases/mozilla-aurora/rev/7546fedad918
BuildID   20140914160203
Version   34.0a2
ro.build.date   Fri Jun 27 15:57:58 CST 2014
ro.bootloader   L1TC00011230
ro.build.version.incremental   110


[Result]
PASS

Comment 17

[Sue]

Attachment: video of issue verify

This issue has been verified successfully on Flame 2.1
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        afdfa629be209dd53a1b7b6d6c95eab7077ffcd9
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/dc3018cbdbe6
Build-ID        20141123001201
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141123.035029
FW-Date         Sun Nov 23 03:50:40 EST 2014
Bootloader      L1TC00011880