ical.js gets busted by build minification

RESOLVED FIXED in Firefox OS v2.0

Status

Firefox OS
Gaia::Calendar
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gaye, Assigned: gaye)

Tracking

unspecified
2.0 S5 (4july)
x86
Linux

Firefox Tracking Flags

(blocking-b2g:2.0+, b2g-v2.0 fixed, b2g-v2.1 fixed)

Details

(Whiteboard: [p=1])

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
This is something that :mcav brought up to me last week.

[JavaScript Error: "SyntaxError: invalid regular expression flag v" {file: "app://calendar.gaiamobile.org/js/ext/ical.js?time=1401686258065" line: 264 column: 60 source: "=/^([+-])?(5[0-3]|[1-4][0-9]|[1-9])?(SU|MO|TU|WE|TH|FR|SA)$/var ALLOWED_FREQ=['SECONDLY','MINUTELY','HOURLY','DAILY','WE"}]

I'm pretty sure this should break all the calendar things ever, so going to 2.0 nom. Interestingly, that part of the ical codebase hasn't changed in recent years (seriously since November 2012), so my diagnosis is that this work

https://github.com/mozilla-b2g/gaia/pull/19627

which was already backed out for breaking the dialer and usage apps is the culprit. That being said, it seems like from the stack trace we can avoid future issues by adding a semicolon to the end of the RegExp line in ical.js.
(Assignee)

Updated

4 years ago
Assignee: nobody → gaye
(Assignee)

Updated

4 years ago
Whiteboard: [qawanted]
(Assignee)

Comment 2

4 years ago
Adding qawanted to see if this is still reproducing after the build system changes were backed out. Steps to reproduce are basically:

1. Get the latest nightly b2g build.
2. Run the calendar app while collecting adb logs.
3. Try creating a caldav account (like google for instance).

And see if it works and also if

[JavaScript Error: "SyntaxError: invalid regular expression flag v" {file: "app://calendar.gaiamobile.org/js/ext/ical.js?time=1401686258065" line: 264 column: 60 source: "=/^([+-])?(5[0-3]|[1-4][0-9]|[1-9])?(SU|MO|TU|WE|TH|FR|SA)$/var ALLOWED_FREQ=['SECONDLY','MINUTELY','HOURLY','DAILY','WE"}]

pops up in the logs.
(Assignee)

Comment 3

4 years ago
Created attachment 8446797 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21082

Hey Miller,

There was a build issue caused by a missing semicolon in ical.js. It's been there for a while so I suspect (look up) that the issue stemmed from changes to the build system. Either way, good to cover our bases and I asked qa to look into whether this still reproduces and we need to uplift.
Attachment #8446797 - Flags: review?(mmedeiros)
(Assignee)

Updated

4 years ago
blocking-b2g: --- → 2.0?
Comment on attachment 8446797 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/21082

+1 for using a v0.0.2 instead of "master"!
Attachment #8446797 - Flags: review?(mmedeiros) → review+
(Assignee)

Updated

4 years ago
Whiteboard: [qawanted] → [p=1]
Target Milestone: --- → 2.0 S5 (4july)
(Assignee)

Comment 5

4 years ago
landed on master https://github.com/mozilla-b2g/gaia/commit/d7a6aedf56ca7b1075e7aa69557dc368a60cffe6
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
(In reply to Gareth Aye [:gaye] from comment #2)
> Adding qawanted to see if this is still reproducing after the build system
> changes were backed out. Steps to reproduce are basically:
> 
> 1. Get the latest nightly b2g build.
> 2. Run the calendar app while collecting adb logs.
> 3. Try creating a caldav account (like google for instance).
> 
> And see if it works and also if
> 
> [JavaScript Error: "SyntaxError: invalid regular expression flag v" {file:
> "app://calendar.gaiamobile.org/js/ext/ical.js?time=1401686258065" line: 264
> column: 60 source:
> "=/^([+-])?(5[0-3]|[1-4][0-9]|[1-9])?(SU|MO|TU|WE|TH|FR|SA)$/var
> ALLOWED_FREQ=['SECONDLY','MINUTELY','HOURLY','DAILY','WE"}]

Gareth, what exactly does this message mean? Can you explain what happens, does the user lose of events or he can't add anythin on the calendar etc, how exactly is this busted ?
> 
> pops up in the logs.
Flags: needinfo?(gaye)
(Assignee)

Comment 7

4 years ago
Excerpt from irc chat with :pdol about this

12:22 PM <gaye> So for a couple weeks the ical.js build was busted which meant that no synchronizing of caldav stuff worked
12:22 PM <gaye> I believe this was due to a change to the build system that also broke some comms apps which has since been backed out
12:23 PM <gaye> That being said, there is 0 risk to taking the patch and the change made is good as far as preventing future hiccups on 2.0 like this go
12:24 PM <gaye> However it happens, I would like to uplift it since it applies cleanly and there's no risk and it's a safeguard for future build system changes being uplifted to 2.0
Flags: needinfo?(gaye)

Updated

4 years ago
blocking-b2g: 2.0? → 2.0+
package.json doesn't exist on v2.0. Is that going to break things if that change is ignored on the uplift?
status-b2g-v2.0: --- → affected
status-b2g-v2.1: --- → fixed
Flags: needinfo?(gaye)
(Assignee)

Comment 9

4 years ago
Shouldn't be an issue if package.json doesn't get cherry-picked, but thanks for bringing it up!
Flags: needinfo?(gaye)
You need to log in before you can comment on or make changes to this bug.