Skip to content

Commit

Permalink
Merge pull request #16 from duckduckgo/shane/fix-display-name
Browse files Browse the repository at this point in the history
fix: display name when entityName matches eTLD+1
  • Loading branch information
shakyShane authored Dec 3, 2022
2 parents 3431005 + 789cd22 commit ebf76af
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 2 deletions.
7 changes: 6 additions & 1 deletion build/app/public/js/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -30547,7 +30547,12 @@ var AggregatedCompanyResponseData = /*#__PURE__*/function () {
var displayName;
var urlHostname = hostname.replace(/^www\./, '');
if (request.entityName) {
displayName = (0, _normalizeCompanyNameEs.removeTLD)(request.entityName);
// if 'entityName' + 'eTLDplus1' match, just use 'request.eTLDplus1' un-modified
if (request.entityName === request.eTLDplus1) {
displayName = request.eTLDplus1;
} else {
displayName = (0, _normalizeCompanyNameEs.removeTLD)(request.entityName);
}
} else {
displayName = request.eTLDplus1 || request.url;
}
Expand Down
21 changes: 21 additions & 0 deletions shared/js/browser/utils/__snapshots__/request-details.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,24 @@ Array [
},
]
`;

exports[`RequestDetails uses correct display name with EntityName matches ETLD+1 1`] = `
AggregatedCompanyResponseData {
"entities": Object {
"bbci.co.uk": AggregateCompanyData {
"displayName": "bbci.co.uk",
"name": undefined,
"normalizedName": "bbcico",
"prevalence": 0,
"urls": Object {
"static.files.bbci.co.uk": TrackerUrl {
"category": undefined,
"url": "static.files.bbci.co.uk",
},
},
},
},
"entitiesCount": 1,
"requestCount": 1,
}
`;
7 changes: 6 additions & 1 deletion shared/js/browser/utils/request-details.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,12 @@ export class AggregatedCompanyResponseData {
const urlHostname = hostname.replace(/^www\./, '')

if (request.entityName) {
displayName = removeTLD(request.entityName)
// if 'entityName' + 'eTLDplus1' match, just use 'request.eTLDplus1' un-modified
if (request.entityName === request.eTLDplus1) {
displayName = request.eTLDplus1
} else {
displayName = removeTLD(request.entityName)
}
} else {
displayName = request.eTLDplus1 || request.url
}
Expand Down
27 changes: 27 additions & 0 deletions shared/js/browser/utils/request-details.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,4 +65,31 @@ describe('RequestDetails', () => {
const state = requestDetails.state(true)
expect(state).toBe(states.protectionsOn_blocked)
})
/**
* In this case, 'entityName' and 'eTLDplus1' match - our logic
* was always running `removeTLD` when the entity name was present.
* This behaviour was intended to prevent Entity names like "Amazon.com" from displaying,
* but it accidentally also stripped `.uk` from the example below.
*
* This test ensures that the following is true:
* "displayName": "bbci.co.uk"
*/
it('uses correct display name with EntityName matches ETLD+1', () => {
const requestDetails = fromJson({
requests: [
{
pageUrl: 'https://www.bbc.co.uk/',
state: {
allowed: {
reason: 'otherThirdPartyRequest',
},
},
url: 'https://static.files.bbci.co.uk/core/bundle-defaultVendors.99d08071a5c40316d056.js',
eTLDplus1: 'bbci.co.uk',
entityName: 'bbci.co.uk',
},
],
})
expect(requestDetails.all).toMatchSnapshot()
})
})

0 comments on commit ebf76af

Please sign in to comment.