Shu Yang (Gerrit)

unread,

Dec 24, 2025, 4:07:48 AM (3 days ago) Dec 24

to Wenyu Fu, Sirisha Kavuluru, Chromium LUCI CQ, chromium...@chromium.org

Attention needed from Sirisha Kavuluru and Wenyu Fu

Shu Yang added 1 comment

Patchset-level comments

File-level comment, Patchset 2 (Latest):

Shu Yang . resolved

PTAL

Open in Gerrit

Related details

Attention is currently required from:

Sirisha Kavuluru
Wenyu Fu

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Wenyu Fu (Gerrit)

unread,

1:36 PM (10 hours ago) 1:36 PM

to Shu Yang, Sirisha Kavuluru, Chromium LUCI CQ, chromium...@chromium.org

Attention needed from Shu Yang and Sirisha Kavuluru

Wenyu Fu voted and added 1 comment

Votes added by Wenyu Fu

Code-Review

+1

1 comment

File chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java

Line 804, Patchset 2 (Latest): boolean isPdfPage = nativePage != null && nativePage.isPdf();

Wenyu Fu . unresolved

optional nit:

I personally found these condition quite complex. It might be a case where early return can help leverage some of the cognitive load for understanding this.

suggestion:

```
if (!canShareTab) return;

if (!isPrintingEnabled) return;

if (!isPdfPage) return;

return (DeviceInfo.isDesktop() || !isChromeNativeScheme)

```

Open in Gerrit

Related details

Attention is currently required from:

Shu Yang
Sirisha Kavuluru

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Shu Yang (Gerrit)

unread,

6:18 PM (5 hours ago) 6:18 PM

to Wenyu Fu, Sirisha Kavuluru, Chromium LUCI CQ, chromium...@chromium.org

Attention needed from Sirisha Kavuluru

Shu Yang added 1 comment

File chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java

Line 804, Patchset 2: boolean isPdfPage = nativePage != null && nativePage.isPdf();

Wenyu Fu . resolved

optional nit:
I personally found these condition quite complex. It might be a case where early return can help leverage some of the cognitive load for understanding this.
suggestion:
```
if (!canShareTab) return;
if (!isPrintingEnabled) return;
if (!isPdfPage) return;
return (DeviceInfo.isDesktop() || !isChromeNativeScheme)
```

Shu Yang

Done

Open in Gerrit

Related details

Attention is currently required from:

Sirisha Kavuluru

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Shu Yang (Gerrit)

unread,

7:13 PM (4 hours ago) 7:13 PM

to Wenyu Fu, Sirisha Kavuluru, Chromium LUCI CQ, chromium...@chromium.org

Attention needed from Sirisha Kavuluru

Shu Yang voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention is currently required from:

Sirisha Kavuluru

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

7:17 PM (4 hours ago) 7:17 PM

to Shu Yang, Wenyu Fu, Sirisha Kavuluru, chromium...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

2 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java
Insertions: 18, Deletions: 11.

@@ -792,24 +792,31 @@
 
         // Check if sharing (which includes printing) is generally enabled for this tab's content.
         boolean canShareTab = ShareUtils.shouldEnableShare(currentTab);
+        if (!canShareTab) {
+            return false;
+        }
 
         // Check if printing is specifically enabled in user preferences for the current profile.
         Profile profile = currentTab.getProfile();
         boolean isPrintingEnabled = UserPrefs.get(profile).getBoolean(Pref.PRINTING_ENABLED);
+        if (!isPrintingEnabled) {
+            return false;
+        }
 
-        // The print functionality should be displayed in the app menu if the device is Desktop
-        // Android or if the current tab contains a transient PDF page. Printing for locally stored
-        // PDFs is not yet supported.
+        // The print functionality is enabled if:
+        // 1. The device is running Desktop Android, OR
+        // 2. The current tab is a web-based PDF (transient).
+        // Note: Printing local PDFs (chrome-native://) is not yet supported.
         NativePage nativePage = currentTab.getNativePage();
-        boolean isPdfPage = nativePage != null && nativePage.isPdf();
-        boolean isChromeNativeScheme =
-                currentTab.getUrl().getScheme().equals(UrlConstants.CHROME_NATIVE_SCHEME);
-        boolean isTransientPdfPage = isPdfPage && !isChromeNativeScheme;
-        boolean isLocalPdfPage = isPdfPage && isChromeNativeScheme;
-        boolean isEligibleForPrint =
-                !isLocalPdfPage && (DeviceInfo.isDesktop() || isTransientPdfPage);
+        boolean isPdf = nativePage != null && nativePage.isPdf();
+        boolean isLocalPdf =
+                isPdf && currentTab.getUrl().getScheme().equals(UrlConstants.CHROME_NATIVE_SCHEME);
 
-        return canShareTab && isPrintingEnabled && isEligibleForPrint;
+        if (isLocalPdf) {
+            return false;
+        }
+
+        return DeviceInfo.isDesktop() || isPdf;
     }
 
     private MVCListAdapter.ListItem buildPrintItem(Tab currentTab) {
```

Change information

Commit message:

Do not show print in app menu for locally stored PDFs

Bug: 463882238

Change-Id: I4851a130aa2e7288ce385167576ffb94060d6cc7

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7318746

Commit-Queue: Shu Yang <shu...@google.com>

Reviewed-by: Wenyu Fu <wen...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1562810}

Files:

M chrome/android/java/src/org/chromium/chrome/browser/tabbed_mode/TabbedAppMenuPropertiesDelegate.java

Change size: S

Delta: 1 file changed, 18 insertions(+), 5 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Wenyu Fu

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Do not show print in app menu for locally stored PDFs [chromium/src : main]

Shu Yang (Gerrit)

Shu Yang added 1 comment

Related details

Wenyu Fu (Gerrit)

Wenyu Fu voted and added 1 comment

Votes added by Wenyu Fu

1 comment

Related details

Shu Yang (Gerrit)

Shu Yang added 1 comment

Related details

Shu Yang (Gerrit)

Shu Yang voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information