Hey Drew, I have a couple questions related to the bug. I think the algorithm we talked about for `stripURLPrefix` is fine, but I am running up into some issues with the tests:
When I add `localhost` to the history via `PlacesTestUtils.addVisits`, I'm expecting only to see one result but I'm seeing a second one specifically for `lo` and `localhost`. I attempt to run `check_results` with the following:
```js
add_task(async function originLooksLikePrefix() {
let hostAndPort = "localhost:8888";
let address = `http://${hostAndPort}/`;
await PlacesTestUtils.addVisits([{ uri: address }]);
for (let search of ["lo", "localhost", "localhost:", "localhost:8888"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
autofilled: hostAndPort + "/",
completed: address,
matches: [
makeVisitResult(context, {
uri: address,
title: hostAndPort,
heuristic: true,
}),
],
});
}
await cleanup();
});
```
Here is a dump of `context.results` when the test fails on the first and second example:
```js
```
Bug 1560676 Comment 8 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Hey Drew, I have a couple questions related to the bug. I think the algorithm we talked about for `stripURLPrefix` is fine, but I am running up into some issues with the tests:
When I add `localhost` to the history via `PlacesTestUtils.addVisits`, I'm expecting only to see one result but I'm seeing a second one specifically for `lo` and `localhost`. I attempt to run `check_results` with the following:
```js
add_task(async function originLooksLikePrefix() {
let hostAndPort = "localhost:8888";
let address = `http://${hostAndPort}/`;
await PlacesTestUtils.addVisits([{ uri: address }]);
for (let search of ["lo", "localhost", "localhost:", "localhost:8888"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
autofilled: hostAndPort + "/",
completed: address,
matches: [
makeVisitResult(context, {
uri: address,
title: hostAndPort,
heuristic: true,
}),
],
});
}
await cleanup();
});
```
Here is a dump of `context.results` when the test fails on `lo` and `localhost` example:
```js
[{
"type ": 3,
"source ": 2,
"rowIndex ": -1,
"heuristic ": true,
"payload": {
"title": "localhost:8888",
"url": "http://localhost:8888/",
"icon": "page-icon:http://localhost:8888/",
"displayUrl": "http://localhost:8888"
},
"payloadHighlights": {
"title": [
[0, 2]
],
"url": [
[7, 2]
],
"displayUrl": [
[7, 2]
],
"icon": []
},
"autofill": {
"value": "localhost:8888/",
"selectionStart": 2,
"selectionEnd": 15
},
"providerName": "Autofill",
"providerType": 1
}, {
"type": 8,
"source": 3,
"rowIndex": -1,
"heuristic": false,
"payload": {
"engine": "Suggestions",
"url": "localhost",
"providesSearchMode": true,
"icon": "chrome://global/skin/icons/search-glass.svg",
"dynamicType": "onboardTabToSearch",
"satisfiesAutofillThreshold": false
},
"payloadHighlights": {
"engine": [],
"url": [],
"providesSearchMode": [],
"icon": [],
"dynamicType": [],
"satisfiesAutofillThreshold": []
},
"resultSpan": 2,
"suggestedIndex": 1,
"providerName": "TabToSearch",
"providerType": 2
}]
```
Note that if I change `localhost:8888` to something gibberish with a port e.g. `mocalhost:8888`, and change the strings in the array to some variant of `mocalhost`, the tests pass, which implies a different treatment for `localhost` somewhere along the way?
Additionally, for testing the URI safelist, history seems nuanced. It seems like there's a back/forward history and a History that's managed by `History.jsm`? So a user can go back and forth between non-`https` pages and `https` pages using the back/forward functionality, but they won't see those pages in the list of urls from `History`, which is relevant since the tests use `insertMany` before entering terms in the URL bar. Tossing a non-standard URI will result in `No items were added to history.` error.
So I'm assuming that for `javascript` , `data-url`, and `view-source`, I'm going to have to add a bookmark. This is how I was able to make tests pass but I believe I'm missing the point of not including `autofilled`?:
```js
add_task(async function nonStandardBookmarkPrefix() {
Services.prefs.setBoolPref("browser.urlbar.filter.javascript", false);
let address = "javascript:(()=>())();";
let title = "Bookmarklet";
await PlacesTestUtils.addBookmarkWithDetails({
uri: address,
title,
});
for (let search of ["ja", "javascript"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeSearchResult(context, {
query: search,
engineName: Services.search.defaultEngine.name,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
for (let search of ["javascript:(", "javascript:(()=>())();"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeVisitResult(context, {
uri: search,
source: UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
title: search,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
await cleanupPlaces();
});
```
Hey Drew, I have a couple questions related to the bug. I think the algorithm we talked about for `stripURLPrefix` is fine, but I am running up into some issues with the tests:
When I add `localhost` to the history via `PlacesTestUtils.addVisits`, I'm expecting only to see one result but I'm seeing a second one specifically for `lo` and `localhost`. I attempt to run `check_results` with the following:
```
add_task(async function originLooksLikePrefix() {
let hostAndPort = "localhost:8888";
let address = `http://${hostAndPort}/`;
await PlacesTestUtils.addVisits([{ uri: address }]);
for (let search of ["lo", "localhost", "localhost:", "localhost:8888"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
autofilled: hostAndPort + "/",
completed: address,
matches: [
makeVisitResult(context, {
uri: address,
title: hostAndPort,
heuristic: true,
}),
],
});
}
await cleanup();
});
```
Here is a dump of `context.results` when the test fails on `lo` and `localhost` example:
```
[{
"type ": 3,
"source ": 2,
"rowIndex ": -1,
"heuristic ": true,
"payload": {
"title": "localhost:8888",
"url": "http://localhost:8888/",
"icon": "page-icon:http://localhost:8888/",
"displayUrl": "http://localhost:8888"
},
"payloadHighlights": {
"title": [
[0, 2]
],
"url": [
[7, 2]
],
"displayUrl": [
[7, 2]
],
"icon": []
},
"autofill": {
"value": "localhost:8888/",
"selectionStart": 2,
"selectionEnd": 15
},
"providerName": "Autofill",
"providerType": 1
}, {
"type": 8,
"source": 3,
"rowIndex": -1,
"heuristic": false,
"payload": {
"engine": "Suggestions",
"url": "localhost",
"providesSearchMode": true,
"icon": "chrome://global/skin/icons/search-glass.svg",
"dynamicType": "onboardTabToSearch",
"satisfiesAutofillThreshold": false
},
"payloadHighlights": {
"engine": [],
"url": [],
"providesSearchMode": [],
"icon": [],
"dynamicType": [],
"satisfiesAutofillThreshold": []
},
"resultSpan": 2,
"suggestedIndex": 1,
"providerName": "TabToSearch",
"providerType": 2
}]
```
Note that if I change `localhost:8888` to something gibberish with a port e.g. `mocalhost:8888`, and change the strings in the array to some variant of `mocalhost`, the tests pass, which implies a different treatment for `localhost` somewhere along the way?
Additionally, for testing the URI safelist, history seems nuanced. It seems like there's a back/forward history and a History that's managed by `History.jsm`? So a user can go back and forth between non-`https` pages and `https` pages using the back/forward functionality, but they won't see those pages in the list of urls from `History`, which is relevant since the tests use `insertMany` before entering terms in the URL bar. Tossing a non-standard URI will result in `No items were added to history.` error.
So I'm assuming that for `javascript` , `data-url`, and `view-source`, I'm going to have to add a bookmark. This is how I was able to make tests pass but I believe I'm missing the point of not including `autofilled`?:
```
add_task(async function nonStandardBookmarkPrefix() {
Services.prefs.setBoolPref("browser.urlbar.filter.javascript", false);
let address = "javascript:(()=>())();";
let title = "Bookmarklet";
await PlacesTestUtils.addBookmarkWithDetails({
uri: address,
title,
});
for (let search of ["ja", "javascript"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeSearchResult(context, {
query: search,
engineName: Services.search.defaultEngine.name,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
for (let search of ["javascript:(", "javascript:(()=>())();"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeVisitResult(context, {
uri: search,
source: UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
title: search,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
await cleanupPlaces();
});
```
Hey Drew, I have a couple questions related to the bug. I think the algorithm we talked about for `stripURLPrefix` is fine, but I am running up into some issues with the tests:
When I add `localhost` to the history via `PlacesTestUtils.addVisits`, I'm expecting only to see one result but I'm seeing a second one specifically for `lo` and `localhost`. I attempt to run `check_results` with the following:
```js add_task(async function originLooksLikePrefix() {
let hostAndPort = "localhost:8888";
let address = `http://${hostAndPort}/`;
await PlacesTestUtils.addVisits([{ uri: address }]);
for (let search of ["lo", "localhost", "localhost:", "localhost:8888"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
autofilled: hostAndPort + "/",
completed: address,
matches: [
makeVisitResult(context, {
uri: address,
title: hostAndPort,
heuristic: true,
}),
],
});
}
await cleanup();
});
```
Here is a dump of `context.results` when the test fails on `lo` and `localhost` example:
```js [{
"type ": 3,
"source ": 2,
"rowIndex ": -1,
"heuristic ": true,
"payload": {
"title": "localhost:8888",
"url": "http://localhost:8888/",
"icon": "page-icon:http://localhost:8888/",
"displayUrl": "http://localhost:8888"
},
"payloadHighlights": {
"title": [
[0, 2]
],
"url": [
[7, 2]
],
"displayUrl": [
[7, 2]
],
"icon": []
},
"autofill": {
"value": "localhost:8888/",
"selectionStart": 2,
"selectionEnd": 15
},
"providerName": "Autofill",
"providerType": 1
}, {
"type": 8,
"source": 3,
"rowIndex": -1,
"heuristic": false,
"payload": {
"engine": "Suggestions",
"url": "localhost",
"providesSearchMode": true,
"icon": "chrome://global/skin/icons/search-glass.svg",
"dynamicType": "onboardTabToSearch",
"satisfiesAutofillThreshold": false
},
"payloadHighlights": {
"engine": [],
"url": [],
"providesSearchMode": [],
"icon": [],
"dynamicType": [],
"satisfiesAutofillThreshold": []
},
"resultSpan": 2,
"suggestedIndex": 1,
"providerName": "TabToSearch",
"providerType": 2
}]
```
Note that if I change `localhost:8888` to something gibberish with a port e.g. `mocalhost:8888`, and change the strings in the array to some variant of `mocalhost`, the tests pass, which implies a different treatment for `localhost` somewhere along the way?
Additionally, for testing the URI safelist, history seems nuanced. It seems like there's a back/forward history and a History that's managed by `History.jsm`? So a user can go back and forth between non-`https` pages and `https` pages using the back/forward functionality, but they won't see those pages in the list of urls from `History`, which is relevant since the tests use `insertMany` before entering terms in the URL bar. Tossing a non-standard URI will result in `No items were added to history.` error.
So I'm assuming that for `javascript` , `data-url`, and `view-source`, I'm going to have to add a bookmark. This is how I was able to make tests pass but I believe I'm missing the point of not including `autofilled`?:
```js add_task(async function nonStandardBookmarkPrefix() {
Services.prefs.setBoolPref("browser.urlbar.filter.javascript", false);
let address = "javascript:(()=>())();";
let title = "Bookmarklet";
await PlacesTestUtils.addBookmarkWithDetails({
uri: address,
title,
});
for (let search of ["ja", "javascript"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeSearchResult(context, {
query: search,
engineName: Services.search.defaultEngine.name,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
for (let search of ["javascript:(", "javascript:(()=>())();"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeVisitResult(context, {
uri: search,
source: UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
title: search,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
await cleanupPlaces();
});
```
```
Hey Drew, I have a couple questions related to the bug. I think the algorithm we talked about for `stripURLPrefix` is fine, but I am running up into some issues with the tests:
When I add `localhost` to the history via `PlacesTestUtils.addVisits`, I'm expecting only to see one result but I'm seeing a second one specifically for `lo` and `localhost`. I attempt to run `check_results` with the following:
```js add_task(async function originLooksLikePrefix() {
let hostAndPort = "localhost:8888";
let address = `http://${hostAndPort}/`;
await PlacesTestUtils.addVisits([{ uri: address }]);
for (let search of ["lo", "localhost", "localhost:", "localhost:8888"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
autofilled: hostAndPort + "/",
completed: address,
matches: [
makeVisitResult(context, {
uri: address,
title: hostAndPort,
heuristic: true,
}),
],
});
}
await cleanup();
});
```
Here is a dump of `context.results` when the test fails on `lo` and `localhost` example:
```js [{
"type ": 3,
"source ": 2,
"rowIndex ": -1,
"heuristic ": true,
"payload": {
"title": "localhost:8888",
"url": "http://localhost:8888/",
"icon": "page-icon:http://localhost:8888/",
"displayUrl": "http://localhost:8888"
},
"payloadHighlights": {
"title": [
[0, 2]
],
"url": [
[7, 2]
],
"displayUrl": [
[7, 2]
],
"icon": []
},
"autofill": {
"value": "localhost:8888/",
"selectionStart": 2,
"selectionEnd": 15
},
"providerName": "Autofill",
"providerType": 1
}, {
"type": 8,
"source": 3,
"rowIndex": -1,
"heuristic": false,
"payload": {
"engine": "Suggestions",
"url": "localhost",
"providesSearchMode": true,
"icon": "chrome://global/skin/icons/search-glass.svg",
"dynamicType": "onboardTabToSearch",
"satisfiesAutofillThreshold": false
},
"payloadHighlights": {
"engine": [],
"url": [],
"providesSearchMode": [],
"icon": [],
"dynamicType": [],
"satisfiesAutofillThreshold": []
},
"resultSpan": 2,
"suggestedIndex": 1,
"providerName": "TabToSearch",
"providerType": 2
}]
```
Note that if I change `localhost:8888` to something gibberish with a port e.g. `mocalhost:8888`, and change the strings in the array to some variant of `mocalhost`, the tests pass, which implies a different treatment for `localhost` somewhere along the way?
Additionally, for testing the URI safelist, history seems nuanced. It seems like there's a back/forward history and a History that's managed by `History.jsm`? So a user can go back and forth between non-`https` pages and `https` pages using the back/forward functionality, but they won't see those pages in the list of urls from `History`, which is relevant since the tests use `insertMany` before entering terms in the URL bar. Tossing a non-standard URI will result in `No items were added to history.` error.
So I'm assuming that for `javascript` , `data-url`, and `view-source`, I'm going to have to add a bookmark. This is how I was able to make tests pass but I believe I'm missing the point of not including `autofilled`?:
```js add_task(async function nonStandardBookmarkPrefix() {
Services.prefs.setBoolPref("browser.urlbar.filter.javascript", false);
let address = "javascript:(()=>())();";
let title = "Bookmarklet";
await PlacesTestUtils.addBookmarkWithDetails({
uri: address,
title,
});
for (let search of ["ja", "javascript"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeSearchResult(context, {
query: search,
engineName: Services.search.defaultEngine.name,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
for (let search of ["javascript:(", "javascript:(()=>())();"]) {
let context = createContext(search, { isPrivate: false });
await check_results({
context,
completed: address,
matches: [
makeVisitResult(context, {
uri: search,
source: UrlbarUtils.RESULT_SOURCE.OTHER_LOCAL,
title: search,
heuristic: true,
}),
makeBookmarkResult(context, {
uri: address,
title: "Bookmarklet",
iconUri: "chrome://global/skin/icons/defaultFavicon.svg",
}),
],
});
}
await cleanupPlaces();
});
```
Modifying comment because it's acting a bit odd.