Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Bug 796869 - [contacts] Get ready for CSP #5637

Merged
merged 1 commit into from Oct 4, 2012

Conversation

arcturus
Copy link
Contributor

@arcturus arcturus commented Oct 2, 2012

Removing inline javascript from contacts app.

Still pending the fb part, to be done in a follow up

/cc @albertopq to r? in bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=796869

@albertopq
Copy link
Contributor

/botio lint

@ghost
Copy link

ghost commented Oct 3, 2012

From: Bot.io (Main)


Received

Command cmd_lint from @albertopq received. Current queue size: 0

Live output at: http://50.116.11.35:8000/51470d7e681f801/output.txt

@ghost
Copy link

ghost commented Oct 3, 2012

From: Bot.io (Main)


Failed

Full output at http://50.116.11.35:8000/51470d7e681f801/output.txt

Total script time: 1.86 mins

Lint: FAILED
----Skipping 88 file(s).
----- /apps/browser/js/browser.js
58, E:0110: Line too long (84 characters).
----- /apps/calculator/js/calculator.js
55, E:0002: Missing space before "("
56, E:0002: Missing space before "("
58, E:0002: Missing space before "("
67, E:0002: Missing space before "("
69, E:0002: Missing space before "("
----- /apps/calendar/js/views/day.js
23, E:0110: Line too long (84 characters).
----- /apps/camera/js/camera.js
202, E:0110: Line too long (94 characters).
205, E:0110: Line too long (96 characters).
243, E:0131: Single-quoted string preferred over double-quoted string.
----- /apps/email/js/message-cards.js
326, E:0121: Illegal comma at end of object literal
351, E:0002: Missing space before "("
787, E:0121: Illegal comma at end of object literal
788, E:0121: Illegal comma at end of object literal
----- /apps/email/js/setup-cards.js
253, E:0001: Extra space after ":"
254, E:0001: Extra space after ":"
256, E:0001: Extra space after ":"
259, E:0001: Extra space after ":"
260, E:0001: Extra space after ":"
262, E:0001: Extra space after ":"
----- /apps/settings/js/icc.js
6, E:0002: Missing space before "{"
13, E:0131: Single-quoted string preferred over double-quoted string.
126, E:0110: Line too long (82 characters).
127, E:0001: Extra space after "function"
174, E:0001: Extra space after "function"
----- /apps/settings/js/phone_lock.js
119, E:0110: Line too long (81 characters).
----- /apps/settings/js/simcard_dialog.js
43, E:0001: Extra space after "function"
----- /apps/settings/js/utils.js
124, E:0013: No semicolon is required to end a code block
----- /apps/sms/js/background.js
77, E:0001: Extra space at end of line
----- /apps/system/js/window_manager.js
145, E:0110: Line too long (82 characters).
167, E:0110: Line too long (82 characters).
762, E:0110: Line too long (81 characters).
812, E:0110: Line too long (81 characters).
814, E:0110: Line too long (85 characters).
1214, E:0002: Missing space before "("
----- /apps/system/js/wrapper.js
34, E:0002: Missing space before "("
42, E:0002: Missing space before "("
----- /apps/system/test/unit/date_picker_test.js
94, E:0110: Line too long (81 characters).
Found 38 errors, including 0 new errors, in 14 files (354 files OK).

Some of the errors reported by GJsLint may be auto-fixable using the script
fixjsstyle. Please double check any changes it makes and report any bugs. The
script can be run by executing:

fixjsstyle --nojsdoc -r apps -e homescreen/everything.me,sms/js/ext,pdfjs/content,pdfjs/test,email/js/ext,music/js/ext,calendar/js/ext
make: *** [lint] Error 1

@albertopq
Copy link
Contributor

r+ from my side. I've been trying it in the otoro and I can't see any problem.

@@ -567,10 +568,86 @@ var Contacts = (function() {
loading.classList.remove('show-overlay');
};

var initEventListeners = function initEventListener() {
// Activity (any) cancellation
document.getElementById('cancel_activty').addEventListener('click',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have something more generic and easy to be read like

var listeners = [
{
selector: 'mySelector',
handler: myFunction
}
]

@arcturus
Copy link
Contributor Author

arcturus commented Oct 3, 2012

@jmcanterafonseca @albertopq updated, r?

}

// Add extra fields to edit mode
var addElementButtons = document.querySelectorAll('#contact-form > button');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the array above declares selectors i.e. '#xx'' instead of ids, we could remove this code and use the same mechanism as above, right? of course we would need to change also getElementById by querySelectorAll but that approach will save us from some lines of code

@arcturus
Copy link
Contributor Author

arcturus commented Oct 4, 2012

@jmcanterafonseca r? (patch v3 sent to bugzilla)

@arcturus
Copy link
Contributor Author

arcturus commented Oct 4, 2012

@jmcanterafonseca last r? (fingers crossed is the last one), sorry for the many updates!

@arcturus
Copy link
Contributor Author

arcturus commented Oct 4, 2012

Got the r+ from @jmcanterafonseca in bugzilla, merging

arcturus added a commit that referenced this pull request Oct 4, 2012
Bug 796869 - [contacts] Get ready for CSP
@arcturus arcturus merged commit d17fb1f into mozilla-b2g:master Oct 4, 2012
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants