Bug 808946 - Escape HTML on Contacts app r=gwagner #6808

Merged
merged 1 commit into from Dec 6, 2012

Projects

None yet

3 participants

@albertopq
Member

Unit/Integration tests passing

@lightsofapollo lightsofapollo and 1 other commented on an outdated diff Dec 4, 2012
...mmunications/contacts/test/unit/contacts_list_test.js
@@ -536,7 +553,7 @@ suite('Render contacts list', function() {
var contact = container.querySelector(selectorContact1);
var img = contact.querySelector('img');
- assert.isTrue(img.getAttribute('backgroundImage') === 'test.png',
+ assert.isTrue(img.dataset.src === 'test.png',
@lightsofapollo
lightsofapollo Dec 4, 2012 Contributor

I am not sure how I ended up looking at this but why is assert.isTrue used over assert.equal ? Error messages are quite a bit better.

@albertopq
albertopq Dec 5, 2012 Member

I'm asking myself the same exact question :) Thanks!

@lightsofapollo lightsofapollo and 1 other commented on an outdated diff Dec 5, 2012
apps/communications/contacts/test/unit/mock_utils.js
@@ -0,0 +1,13 @@
+'use strict';
+
+var MockURL = {
+ createObjectURL: function(url) {
+ console.log("RETURNING " + url);
@lightsofapollo
lightsofapollo Dec 5, 2012 Contributor

Generally you don't want console.log in tests except for debugging (which I expect is why this is here).

@albertopq
albertopq Dec 5, 2012 Member

Ups, missed that debug line. Thanks!

@jmcanterafonseca jmcanterafonseca and 1 other commented on an outdated diff Dec 5, 2012
apps/communications/contacts/js/utilities/normalizer.js
@@ -31,3 +31,15 @@ function normalizeText(value) {
return value;
}
+
+// Taken from /apps/browser/js/browser.js
+var Utils = {
@jmcanterafonseca
jmcanterafonseca Dec 5, 2012 Contributor

as there is already a 'utils' object defined by other modules of the Contacts app I would suggest to reuse it and add the escapeHTML function to that object instead of creating a new one, in this case with the first letter in uppercase

@albertopq
albertopq Dec 6, 2012 Member

Makes sense. I'll fix that

@albertopq albertopq Bug 808946 - Escape HTML on Contacts app
Fixing tests

Fixing printing social icons

Ensuring bday rendering

Fixing tests

Usin 'utils' variable
652e1ab
@albertopq albertopq merged commit 347a8bd into mozilla-b2g:master Dec 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment