Skip to content

Commit

Permalink
[DOM Distiller] added suffix to page title on Android
Browse files Browse the repository at this point in the history
Reader mode pages are opened on a CCT. When using a screen reader the
CCT's title is read out loud after opening. The current implementation
on Android sets the CCT's title as the article's title, with no
indication that the page is on a special mode.

Screen reader users may find the lack of context confusing, as there's
nothing read out loud that indicates that the page is now on a special
mode.

This CL modifies the page's title on Android to include a "Simplified
View" suffix after the article's title. A similar "Reader Mode" suffix
already exists in desktop pages.

Bug: 1418553
Change-Id: Iaf03252a9fe07dfad5a51691c5406bf7dc32cded
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4308297
Commit-Queue: Salvador Guerrero Ramos <salg@google.com>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1114821}
  • Loading branch information
Salvador Guerrero authored and Chromium LUCI CQ committed Mar 8, 2023
1 parent d64c98a commit 5c0a22c
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ public class ReaderModeTest implements CustomMainActivityStart {
public DownloadTestRule mDownloadTestRule = new DownloadTestRule(this);

private static final String TEST_PAGE = "/chrome/test/data/dom_distiller/simple_article.html";
private static final String TITLE = "Test Page Title";
// Suffix added to page titles, string is defined as IDS_DOM_DISTILLER_VIEWER_TITLE_SUFFIX in
// dom_distiller_strings.grdp.
private static final String TITLE_SUFFIX = " - Simplified View";
private static final String PAGE_TITLE = "Test Page Title" + TITLE_SUFFIX;
private static final String CONTENT = "Lorem ipsum";

@SuppressWarnings("FieldCanBeLocal")
Expand Down Expand Up @@ -152,7 +155,7 @@ public void testReaderModeInCCT() throws TimeoutException {
() -> Criteria.checkThat(customTabActivity.getActivityTab(), notNullValue()));
@NonNull
Tab distillerViewerTab = Objects.requireNonNull(customTabActivity.getActivityTab());
waitForDistillation(TITLE, distillerViewerTab);
waitForDistillation(PAGE_TITLE, distillerViewerTab);
}

@Test
Expand All @@ -175,7 +178,7 @@ public void testReaderModeInCCT_Downloaded() throws TimeoutException {
() -> Criteria.checkThat(customTabActivity.getActivityTab(), notNullValue()));
@NonNull
Tab distillerViewerTab = Objects.requireNonNull(customTabActivity.getActivityTab());
waitForDistillation(TITLE, distillerViewerTab);
waitForDistillation(PAGE_TITLE, distillerViewerTab);
}

@Test
Expand Down Expand Up @@ -226,7 +229,7 @@ private CustomTabActivity openReaderModeInIncognitoCCT() throws TimeoutException
() -> Criteria.checkThat(customTabActivity.getActivityTab(), notNullValue()));
@NonNull
Tab distillerViewerTab = Objects.requireNonNull(customTabActivity.getActivityTab());
waitForDistillation(TITLE, distillerViewerTab);
waitForDistillation(PAGE_TITLE, distillerViewerTab);
assertTrue(distillerViewerTab.isIncognito());

return customTabActivity;
Expand Down Expand Up @@ -269,7 +272,7 @@ public void testReaderModeInTab() throws TimeoutException {
TestThreadUtils.runOnUiThreadBlocking(() -> {
tab.getUserDataHost().getUserData(ReaderModeManager.USER_DATA_KEY).activateReaderMode();
});
waitForDistillation(TITLE, mDownloadTestRule.getActivity().getActivityTab());
waitForDistillation(PAGE_TITLE, mDownloadTestRule.getActivity().getActivityTab());
}

@Test
Expand All @@ -286,7 +289,7 @@ public void testPreferenceInCCT() throws TimeoutException {
CriteriaHelper.pollUiThread(() -> customTabActivity.getActivityTab() != null);
@NonNull
Tab distillerViewerTab = Objects.requireNonNull(customTabActivity.getActivityTab());
waitForDistillation(TITLE, distillerViewerTab);
waitForDistillation(PAGE_TITLE, distillerViewerTab);

testPreference(customTabActivity, distillerViewerTab);
}
Expand All @@ -299,11 +302,11 @@ public void testPreferenceInCCT() throws TimeoutException {
"Failing on Lollipop Phone Tester (https://crbug.com/1120830) and test-n-phone (https://crbug.com/1160911)")
public void
testPreferenceInTab() throws TimeoutException {
mDownloadTestRule.loadUrl(
DomDistillerUrlUtils.getDistillerViewUrlFromUrl(DOM_DISTILLER_SCHEME, mURL, TITLE));
mDownloadTestRule.loadUrl(DomDistillerUrlUtils.getDistillerViewUrlFromUrl(
DOM_DISTILLER_SCHEME, mURL, PAGE_TITLE));

Tab tab = mDownloadTestRule.getActivity().getActivityTab();
waitForDistillation(TITLE, tab);
waitForDistillation(PAGE_TITLE, tab);

testPreference(mDownloadTestRule.getActivity(), tab);
}
Expand Down
4 changes: 2 additions & 2 deletions components/dom_distiller/core/viewer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,9 @@ const std::string GetErrorPageJs() {
}

const std::string GetSetTitleJs(std::string title) {
#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
#if BUILDFLAG(IS_IOS)
base::Value suffixValue("");
#else // Desktop
#else // Desktop and Android.
std::string suffix(
l10n_util::GetStringUTF8(IDS_DOM_DISTILLER_VIEWER_TITLE_SUFFIX));
base::Value suffixValue(" - " + suffix);
Expand Down
15 changes: 12 additions & 3 deletions components/dom_distiller_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,17 @@
<message name="IDS_DOM_DISTILLER_WEBUI_TITLE" desc="The title to show on the DOM Distiller debug page.">
DOM Distiller
</message>
<message name="IDS_DOM_DISTILLER_VIEWER_TITLE_SUFFIX" desc="The suffix to show after the page title to indicate we are in reader mode. For example, if the page title was 'An Article', this would be appended to create the title 'An Article - Reader Mode'.">
Reader Mode
</message>
<if expr="is_android">
<then>
<message name="IDS_DOM_DISTILLER_VIEWER_TITLE_SUFFIX" desc="The suffix to show after the page title to indicate we are in reader mode. For example, if the page title was 'An Article', this would be appended to create the title 'An Article - Simplified View'.">
Simplified View
</message>
</then>
<else>
<message name="IDS_DOM_DISTILLER_VIEWER_TITLE_SUFFIX" desc="The suffix to show after the page title to indicate we are in reader mode. For example, if the page title was 'An Article', this would be appended to create the title 'An Article - Reader Mode'.">
Reader Mode
</message>
</else>
</if>

</grit-part>

0 comments on commit 5c0a22c

Please sign in to comment.