WordPress-Android CMM-1054 when opening an unauthorized post, the three dots menu icon is not working

Description

Problem

When opening a deeplink to a Reader post the user isn't authorized to view:

The error message said "Use the top action button to try a browser instead"
The three-dots menu was visible but did nothing when tapped (
The browse icon was hidden

Solution

Hide all toolbar menu items in error state (they require a loaded post to function)
Add "Open in browser" button below the error message when opened via deeplink
Update error message strings to remove references to the non-existent "top action button"

Testing instructions

While being unlogged to WP.COM, spen a deeplink to a private/restricted P2

[ ] Verify error message displays with "Open in browser" button (no toolbar menu icons), and it opens the browser

log into WP.COM, and open a link to a P2 you are not part of

[ ] Verify error message displays with "Open in browser" button (no toolbar menu icons), and it opens the browser
[ ] Open other posts form the reader or from deeplinks and verify the top menu is shown and works

Before:

After:

Dec 10 '25 15:12 adalpari

1 Warning

:warning: PR is not assigned to a milestone.

	1 Warning
:warning:	PR is not assigned to a milestone.

Generated by :no_entry_sign: Danger

Dec 10 '25 15:12 dangermattic

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Dec 10 '25 15:12 sonarqubecloud[bot]

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22412-a94b641
	Commit	a94b641011e0a920238376358085ba23a982e192
	Direct Download	`jetpack-prototype-build-pr22412-a94b641.apk`

Note: Google Login is not supported on these builds.

Dec 10 '25 15:12 wpmobilebot

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr22412-a94b641
	Commit	a94b641011e0a920238376358085ba23a982e192
	Direct Download	`wordpress-prototype-build-pr22412-a94b641.apk`

Note: Google Login is not supported on these builds.

Dec 10 '25 15:12 wpmobilebot

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 39.09%. Comparing base (edb1fc8) to head (a94b641). :warning: Report is 3 commits behind head on trunk.

Additional details and impacted files

@@           Coverage Diff           @@
##            trunk   #22412   +/-   ##
=======================================
  Coverage   39.09%   39.09%           
=======================================
  Files        2207     2207           
  Lines      106858   106860    +2     
  Branches    15159    15160    +1     
=======================================
+ Hits        41776    41778    +2     
  Misses      61566    61566           
  Partials     3516     3516

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

:snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
:package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Dec 10 '25 15:12 codecov[bot]

@claude

Dec 10 '25 16:12 adalpari

Claude finished @adalpari's task in 1m 40s —— View job

Code Review Complete ✅

I've reviewed the PR changes for fixing CMM-1054. This is a clean, well-implemented bug fix that addresses the issue where the three-dots menu was visible but non-functional in the error state.

Summary

What Changed:

Hides all toolbar menu items when an unauthorized post error occurs
Adds a new "Open in browser" CTA button that appears below the error message when opened via deeplink
Updates error message strings to remove references to the non-existent "top action button"

Files Changed:

ReaderPostDetailFragment.kt: Added hideMenu() function call and "Open in browser" button initialization
ReaderPostDetailViewModel.kt: Added openInBrowserButtonVisibility parameter to ErrorUiState
reader_fragment_post_detail.xml: Added new "Open in browser" button UI
strings.xml: Removed misleading references and added new button text
ReaderPostDetailViewModelTest.kt: Added comprehensive test coverage for the new functionality

Positive Observations

Clean separation of concerns - The error state handling is well-structured with proper visibility control in the ViewModel
Comprehensive test coverage - 4 new unit tests added covering all scenarios (with/without intercepted URI, with/without sign-in offer)
Consistent with existing patterns - The openInBrowserButton follows the same pattern as the existing nux_sign_in_button
User experience improvement - The solution provides a clear alternative action when users can't access a post
Good resource management - Proper use of string resources and no hardcoded strings

Code Quality Observations

ReaderPostDetailFragment.kt (1109-1114)

The hideMenu() function is straightforward and effective:

private fun hideMenu(menu: Menu) {
    menu.findItem(R.id.menu_browse)?.isVisible = false
    menu.findItem(R.id.menu_share)?.isVisible = false
    menu.findItem(R.id.menu_reading_preferences)?.isVisible = false
    menu.findItem(R.id.menu_more)?.isVisible = false
}

Observation: This function manually hides each menu item individually. Consider adding a comment explaining why all items need to be hidden (they require a loaded post to function).

ReaderPostDetailFragment.kt (503-511)

The "Open in browser" button initialization:

private fun initOpenInBrowserButton(view: View) {
    view.findViewById<View>(R.id.open_in_browser_button).setOnClickListener {
        viewModel.interceptedUri?.let { uri ->
            readerTracker.trackUri(AnalyticsTracker.Stat.DEEP_LINKED_FALLBACK, uri)
            ReaderActivityLauncher.openUrl(requireActivity(), uri, OpenUrlType.EXTERNAL)
            requireActivity().finish()
        }
    }
}

Good practices:

Proper null safety with let
Analytics tracking before navigation
Finishes activity after opening browser (appropriate for deeplink fallback scenario)

ReaderPostDetailViewModel.kt (711-719)

The error state construction with proper visibility flags:

private fun trackAndUpdateNotAuthorisedErrorState() {
    trackNotAuthorisedState()
    
    _uiState.value = ErrorUiState(
        message = UiStringRes(getNotAuthorisedErrorMessageRes()),
        signInButtonVisibility = shouldOfferSignIn,
        openInBrowserButtonVisibility = interceptedUri != null
    )
}

Excellent: Clean logic - the browser button only shows when there's actually a URI to open.

Layout Changes (reader_fragment_post_detail.xml:158-171)

<org.wordpress.android.widgets.WPTextView
    android:id="@+id/open_in_browser_button"
    style="@style/WordPress.NUXPrimaryButton"
    android:layout_width="match_parent"
    android:layout_marginTop="@dimen/margin_extra_large"
    android:layout_below="@+id/text_error"
    android:clickable="true"
    android:focusable="true"
    android:gravity="center"
    android:text="@string/reader_open_in_browser"
    android:visibility="gone"
    android:layout_marginEnd="@dimen/reader_detail_margin"
    android:layout_marginStart="@dimen/reader_detail_margin"
    tools:visibility="visible"/>

Concern: Both nux_sign_in_button (line 149) and open_in_browser_button (line 162) use android:layout_below="@+id/text_error". This could cause layout issues if both buttons are visible simultaneously. However, looking at the ViewModel logic, only one should be visible at a time:

Sign-in button: visible when shouldOfferSignIn is true
Browser button: visible when interceptedUri != null

Question: Can both conditions be true simultaneously? If yes, the layout would need adjustment to stack the buttons properly.

Testing Coverage

The test coverage is comprehensive with 4 new tests covering all combinations:

ReaderPostDetailViewModelTest.kt:582-608

✅ Test with intercepted URI → button visible
✅ Test without intercepted URI → button not visible
✅ Tests cover both sign-in scenarios

Strong test coverage ensures the feature works correctly across all states.

String Resources

The string changes appropriately remove misleading references to "top action button":

- "You're not authorized to view this post. Use the top action button to try a browser instead."
+ "You're not authorized to view this post"

- "Try signing in to WordPress.com first or use the top action button to open a browser instead."
+ "Try signing in to WordPress.com first."

New string added:

<string name="reader_open_in_browser">Open in browser</string>

Good: Clear, concise, and user-friendly.

Minor Suggestions

Layout consideration: Verify that the sign-in and browser buttons never appear simultaneously. If they can, adjust the layout to stack them properly using a LinearLayout or proper RelativeLayout chaining.
Documentation: Consider adding a KDoc comment to hideMenu() explaining it's called in error states because menu items require a loaded post.
Accessibility: Both buttons have proper clickable and focusable attributes, but consider adding contentDescription attributes for screen readers.
Edge case: The openInBrowserButton click handler uses requireActivity().finish() - verify this behaves correctly in all navigation scenarios (e.g., when the activity is in a task stack).

Security Considerations

✅ No security issues identified:

The intercepted URI is opened with OpenUrlType.EXTERNAL, which is appropriate for user-initiated browser navigation
Proper null safety prevents crashes
Analytics tracking doesn't expose sensitive data

Performance Considerations

✅ No performance concerns:

Button initialization is lightweight
Menu hiding is a simple visibility toggle
No unnecessary object allocations

Conclusion

This is a well-implemented fix that addresses the reported issue effectively. The code follows Android best practices, maintains consistency with the existing codebase, and includes solid test coverage. The user experience improvement is clear - users now have a working alternative when they can't access a post via deeplink.

Recommendation: ✅ Approve with the minor layout verification noted above.

|

Dec 10 '25 16:12 claude[bot]