New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 736168 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug
Team-Accessibility



Sign in to add a comment

Reader Mode should be available in accessibility mode

Project Member Reported by wychen@chromium.org, Jun 23 2017

Issue description

Chrome Version: M59
OS: Android

Reader Mode (or Mobile-friendly View) on Clank is disabled if the accessibility mode (TalkBack) is on due to issue 503232. Now that Reader Mode is no longer an Overlay Panel after https://codereview.chromium.org/2878543003, it became accessible. Therefore, it should not be disabled anymore.

 

Comment 1 by wychen@chromium.org, Jun 23 2017

WIP CL:
https://codereview.chromium.org/2955533002/

However, it crashes. I haven't figured out why.

Stack Trace:
  RELADDR   FUNCTION                                                                                                FILE:LINE
  00000000                                                                                                          <unknown>
  0008ab2b  Run                                                                                                     /usr/local/google/code/clankium/src/base/callback.h:91
  00095087  RunTask                                                                                                 /usr/local/google/code/clankium/src/base/debug/task_annotator.cc:59
  000ae5cf  RunTask                                                                                                 /usr/local/google/code/clankium/src/base/message_loop/message_loop.cc:422
  000ae82f  DeferOrRunPendingTask                                                                                   /usr/local/google/code/clankium/src/base/message_loop/message_loop.cc:433
  000ae9c3  DoWork                                                                                                  /usr/local/google/code/clankium/src/base/message_loop/message_loop.cc:540
  v------>  DoRunLoopOnce                                                                                           /usr/local/google/code/clankium/src/base/message_loop/message_pump_android.cc:44
  000af9fd  Java_org_chromium_base_SystemMessageHandler_nativeDoRunLoopOnce                                         /usr/local/google/code/clankium/src/out-android-gn/Debug/gen/base/base_jni_headers/base/jni/SystemMessageHandler_jni.h:44
  00067c6d  offset 0x4a000) (void org.chromium.base.SystemMessageHandler.nativeDoRunLoopOnce(long, long, long)+120  /data/data/org.chromium.chrome/incremental-install-files/optimized-dexes/base.base_java.dex.dex
  000681fb  offset 0x4a000) (void org.chromium.base.SystemMessageHandler.handleMessage(android.os.Message)+534      /data/data/org.chromium.chrome/incremental-install-files/optimized-dexes/base.base_java.dex.dex
  037704d5  offset 0x2f51000


The Java-side stack is:

06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at org.chromium.base.SystemMessageHandler.handleMessage(SystemMessageHandler.java:41)
06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at android.os.Handler.dispatchMessage(Handler.java:102)
06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at android.os.Looper.loop(Looper.java:158)
06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at android.app.ActivityThread.main(ActivityThread.java:7225)
06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at java.lang.reflect.Method.invoke(Native Method)
06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1230)
06-22 17:53:04.309 E/cr_SysMessageHandler( 3513):       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1120)

Based on my tests I believe the java stack is unrelated. The native stack is less than helpful. Occasionally I see the following in the log:

06-23 17:43:46.436 18039 18039 I chromium: [INFO:CONSOLE(115)] "DomDistiller debug level: 0", source:  (115)

So it looks like distillation at least starts but fails. I changed the infobar to navigate to a different URL and it doesn't crash, so it looks like a problem with the dom-distiller component.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383

commit 4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383
Author: wychen <wychen@chromium.org>
Date: Fri Jul 07 18:15:36 2017

Add "AllArticles" mode to Reader Mode heuristics

This is a preliminary change to enable the desired mode in the back end.

BUG= 736168 

Review-Url: https://codereview.chromium.org/2961533002
Cr-Commit-Position: refs/heads/master@{#484987}

[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/chrome/browser/about_flags.cc
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/chrome/browser/dom_distiller/distillable_page_utils_browsertest.cc
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/components/dom_distiller/content/renderer/distillability_agent.cc
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/components/dom_distiller/core/dom_distiller_switches.cc
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/components/dom_distiller/core/dom_distiller_switches.h
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/components/dom_distiller/core/experiments.cc
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/components/dom_distiller/core/experiments.h
[modify] https://crrev.com/4b6112cc6c198d8b6df38b3e0a9a1aa19f16b383/ios/chrome/browser/ui/reader_mode/reader_mode_checker.mm

Comment 4 by wychen@chromium.org, Jul 26 2017

Surprisingly the crash mentioned in #c1 no longer reproduces after rebasing to ToT. No idea why.
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6c29838bdd576446894d4fa5a7f19974a8c1b51e

commit 6c29838bdd576446894d4fa5a7f19974a8c1b51e
Author: wychen <wychen@chromium.org>
Date: Thu Jul 27 00:09:08 2017

Do not disable ReaderMode when TalkBack is on

ReaderMode became accessible again after
https://codereview.chromium.org/2878543003. Therefore, it should not
be disabled when TalkBack is turned on anymore.

BUG= 736168 

Review-Url: https://codereview.chromium.org/2955533002
Cr-Commit-Position: refs/heads/master@{#489806}

[modify] https://crrev.com/6c29838bdd576446894d4fa5a7f19974a8c1b51e/chrome/android/java/src/org/chromium/chrome/browser/dom_distiller/ReaderModeManager.java

Labels: triage-android-remaining

Comment 7 by wychen@chromium.org, Sep 29 2017

Status: Fixed (was: Started)

Sign in to add a comment