Skip to content

Commit

Permalink
[Topics] topics-internals: display a warning on invalid classifier input
Browse files Browse the repository at this point in the history
What: In the "Classifier" tab, display a warning if the input string
has a '/' in it.

Why: the user may attempt to type full URLs, but the classifier is only
expecting hostnames. The result topics can appear to be wrong due to
the misuse and misunderstanding.

See https://crbug.com/1325402#c1 for screenshots.

(cherry picked from commit 747b6d3)

Bug: 1325402
Change-Id: If3bed831d14c54531b872bef04d05f000ddbec03
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3647724
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1003269}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3649945
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Yao Xiao <yaoxia@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#31}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
yaoxiachromium authored and Chromium LUCI CQ committed May 16, 2022
1 parent 3e96ad8 commit a0c5e79
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,23 @@ result
return html_content;
}

std::string GetHostsClassificationInputValidationError() {
std::string html_content = EvalJsInWebUI(R"(
let result = '';
let errorsDiv = document.querySelector('#hosts-classification-input-validation-error');
if (errorsDiv.style.display !== 'none') {
Array.from(errorsDiv.children).forEach((errorDiv) => {
result += errorDiv.textContent + '\n';
});
}
result
)");
return html_content;
}

std::string GetHostsClassificationResultTableContent() {
std::string html_content = EvalJsInWebUI(R"(
let result = '';
Expand Down Expand Up @@ -569,6 +586,44 @@ Model file path: /test_path/test_model.tflite
EXPECT_EQ(GetHostsClassificationResultTableContent(),
R"(foo1.com|1. Arts & entertainment;2. Acting & theater;|
foo2.com|3. Comics;4. Concerts & music festivals;5. Dance;|
)");

EXPECT_TRUE(GetHostsClassificationInputValidationError().empty());
}

IN_PROC_BROWSER_TEST_F(BrowsingTopicsInternalsBrowserTest,
ClassifierTab_InvalidInput) {
fixed_browsing_topics_service()->SetWebUIGetBrowsingTopicsStateResultOverride(
browsing_topics::mojom::WebUIGetBrowsingTopicsStateResult::
NewOverrideStatusMessage("Failed to get the topics state."));

// Configure the (mock) model.
test_page_content_annotator_.UsePageTopics(
*optimization_guide::TestModelInfoBuilder()
.SetVersion(1)
.SetModelFilePath(
base::FilePath::FromASCII("/test_path/test_model.tflite"))
.Build(),
{{"foo2.com", TopicsWithUniformWeight({3, 4, 5}, 0.1)},
{"foo1.com", TopicsWithUniformWeight({1, 2}, 0.1)}});

EXPECT_TRUE(ui_test_utils::NavigateToURL(browser(),
GURL(kBrowsingTopicsInternalsUrl)));

constexpr char classify_hosts_script[] = R"(
document.querySelector('#input-hosts-textarea').value = 'foo1.com\nhttps://foo1.com\nfoo1.com/path';
document.querySelector('#hosts-classification-button').click();
)";

EXPECT_TRUE(ExecJs(web_contents()->GetMainFrame(), classify_hosts_script,
content::EXECUTE_SCRIPT_DEFAULT_OPTIONS,
/*world_id=*/1));

EXPECT_TRUE(GetHostsClassificationResultTableContent().empty());

EXPECT_EQ(GetHostsClassificationInputValidationError(),
R"(Host "https://foo1.com" contains invalid character: "/"
Host "foo1.com/path" contains invalid character: "/"
)");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,13 +117,18 @@ button:active {
.model-info-div,
.hosts-classification-input-area-div,
.hosts-classification-result-div,
.hosts-classification-input-validation-error-div,
.features-and-parameters-div,
.classification-input-div {
font-size: 1rem;
line-height: 1.5;
padding: 10px;
}

.hosts-classification-input-validation-error-div {
color: red;
}

.hosts-classification-input-area-div {
border-top: var(--common-border);
margin-top: 10px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ <h3 class="epochs-header">Epochs (latest -> oldest)</h3>
<button type="button" id="hosts-classification-button">Classify</button>
</div>
<div id="hosts-classification-loader-div" class="loader-div"></div>
<div id="hosts-classification-input-validation-error" class="hosts-classification-input-validation-error-div"></div>
<div id="hosts-classification-result-table-wrapper" class="hosts-classification-result-div">
<table id="hosts-classification-result-table" class="hosts-classification-result-table">
<tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,13 +233,6 @@ async function asyncClassifyHosts(hosts: string[], sequenceNumber: Number) {
return;
}

const table = document.querySelector('#hosts-classification-result-table')! as
HTMLTableElement;

while (table.rows[1]) {
table.deleteRow(1);
}

for (let i = 0; i < hosts.length; i++) {
const host = hosts[i] as string;
const topics = topicsForHosts![i] as WebUITopic[];
Expand All @@ -252,6 +245,23 @@ async function asyncClassifyHosts(hosts: string[], sequenceNumber: Number) {
setElementVisible('hosts-classification-result-table-wrapper', true);
}

function clearHostsClassificationResult() {
const table = document.querySelector('#hosts-classification-result-table')! as
HTMLTableElement;

while (table.rows[1]) {
table.deleteRow(1);
}

const div = document.querySelector<HTMLElement>(
'#hosts-classification-input-validation-error');
div!.innerHTML = '';

setElementVisible('hosts-classification-loader-div', false);
setElementVisible('hosts-classification-input-validation-error', false);
setElementVisible('hosts-classification-result-table-wrapper', false);
}

async function asyncGetModelInfo() {
const response = await pageHandler.getModelInfo();

Expand All @@ -277,6 +287,8 @@ async function asyncGetModelInfo() {

document.querySelector(
'#hosts-classification-button')!.addEventListener('click', () => {
clearHostsClassificationResult();

const input = (document.querySelector('#input-hosts-textarea')! as
HTMLTextAreaElement)
.value;
Expand All @@ -292,9 +304,30 @@ async function asyncGetModelInfo() {
preprocessedHosts.push(trimmedHost);
});

setElementVisible('hosts-classification-loader-div', true);
setElementVisible('hosts-classification-result-table-wrapper', false);
asyncClassifyHosts(preprocessedHosts, ++hostsClassificationSequenceNumber);
const inputValidationErrors = [] as string[];
preprocessedHosts.forEach((host) => {
if (host.includes('/')) {
inputValidationErrors.push(
'Host \"' + host + '" contains invalid character: "\/"');
}
});

++hostsClassificationSequenceNumber;

if (inputValidationErrors.length > 0) {
const errorsDiv = document.querySelector<HTMLElement>(
'#hosts-classification-input-validation-error');
inputValidationErrors.forEach((error) => {
const errorDiv = document.createElement('div');
errorDiv.textContent = error;
errorsDiv!.appendChild(errorDiv);
});

setElementVisible('hosts-classification-input-validation-error', true);
} else {
setElementVisible('hosts-classification-loader-div', true);
asyncClassifyHosts(preprocessedHosts, hostsClassificationSequenceNumber);
}
});
}

Expand All @@ -308,6 +341,7 @@ document.addEventListener('DOMContentLoaded', function() {
setElementVisible('model-info-div', false);
setElementVisible('hosts-classification-input-area-div', false);
setElementVisible('hosts-classification-loader-div', false);
setElementVisible('hosts-classification-input-validation-error', false);
setElementVisible('hosts-classification-result-table-wrapper', false);

asyncGetBrowsingTopicsConfiguration();
Expand Down

0 comments on commit a0c5e79

Please sign in to comment.