Disable offline commenting and offline browsing for authenticated users

[westonruter]

There are two scenarios where offline commenting can currently fail due to user authentication:

When a site requires that “Users must be registered and logged in to comment”, when a user does try to submit a comment and it is queued via background sync, it could be that once the user goes back online their session will have expired. At that point, there comment will lost.

Otherwise, even when non-authenticated users can post comments, when an admin user is logged-in, the comment form includes a _wp_unfiltered_html_comment input that has a nonce:

If an admin user submits a comment when offline and the comment is submitted after the session has expired, then if the user had included any privileged HTML in the comment, this will get stripped out by Kses filtering upon submission.

[westonruter]

Offline commenting should probably be turned off entirely for authenticated users.

[westonruter]

Offline browsing overall should probably be turned off for authenticated users, as currently this can cause issues with the admin bar being cached. See #252.

[westonruter]

Another reason to turn off offline browsing for logged-in users: nonce links can fail.

[rutviksavsani]

@westonruter we will be taking this issues further on,
As per my discussion with @ankitrox and @thelovekesh, we have analyzed the following approaches for working on this task.

Initial approaches :

1. Executing user authentication through offline commenting and navigation routing scripts.
   Issue Faced:
   As we have those scripts in Service workers, we would not be able to access Cookies or DOM, which will prevent us to authenticate the users to allow offline commenting for the Non-authenticated users conditionally.

2. Through the Service workers PHP class
   Issue Faced:
   In this case, to serve the same service workers to all the users, the User is set to none while registering the service workers and hence preventing us to authenticate the users to further conditionally load the offline commenting and navigation routing scripts in the Service worker.
   ref: https://github.com/GoogleChromeLabs/pwa-wp/blob/develop/wp-includes/class-wp-service-workers.php#L125-L134

Suggested approach :

As mentioned above in the second approach. we need to conditionally load these scripts mentioned in https://github.com/GoogleChromeLabs/pwa-wp/blob/develop/wp-includes/components/class-wp-service-worker-navigation-routing-component.php#L298-L314
So, to execute it conditionally, we will have to remove this snippet.
which then will allow us to check if the user is authenticated or not and then load the scripts if the user is not authenticated.

Also, another approach would be that,
We can disable the navigation routing component from the service worker __construct method. But there is a use case where third-party plugins/themes can have a problem in determining logged-in status on the wp_{admin/front}_service_worker actions due to wp_set_current_user( 0 ).

[westonruter]

Sorry for delay in being able to respond.

It used to be that the service worker respected the user's authentication state so that it could register user-specific logic. This was eliminated in #279 since it at the time the plugin was configured to automatically reload when a service worker update was detected. This had the extremely annoying behavior of causing a page to reload once the user logged-in, potentially resulting in loss input if the user had started to type something (e.g. in a search box). So in order to make the installed service worker consistent across all authentication states, the service worker was set to run without the user being authenticated in a similar way to the REST API behaves if no nonce is included in the request. This is that snippet you referred to.

Another possibility would be to add logic so that the POST request to add a comment is only added to the background sync queue if either the request has a wordpress_logged_in_* cookie present, or if the POST body has a _wp_unfiltered_html_comment parameter (as output by wp_comment_form_unfiltered_html_nonce()). This nonce approach may be what is needed for #252, to disable offline browsing for logged-in users. But if a user is logged-in should this necessitate them not being able to browse offline?

Lastly, we could also consider just removing the offline commenting functionality entirely. I don't recall this being a feature that anyone has ever used in practice. It should perhaps get removed and go the way of integrations #403 by being deprecated/eliminated. It may be better to bring in background sync later as part of a revamp of commenting in WordPress, where XHR comment submission could be employed instead of full-page submission which would also address the problem of a submitted comment being accidentally lost if the user doesn't hit the back button properly to get back to the form.

[rutviksavsani]

@westonruter
Considering the complete removal of the Offline commenting there is a query:
After removal of the feature, should we provide users an Offline template to the user when he tries to submit a comment while offline on any post?
Things we can consider here are :

1. If we think from an average user's perspective, it is very tough for them to navigate back to the offline site if they are on the browser's default offline page. If we serve an offline template, it serves two purposes.

   • It tells the user that comments can't be submitted as there is no network.
   • He will be able to navigate to some different pages from the offline template itself.

2. The above can be made possible for all the POST requests like the ones coming from a Contact form or some Block by creating a different issue altogether and implementing the solution commonly.

3. No need for templates for POST requests.

Please do share your views on the same.

Attaching an Implementation without the template for Comments form submission.

comment.offline.testing.mp4

[westonruter]

Thank you for the video.

I like the idea of serving back the offline template in response to POST submissions, which we are currently doing actually. You can see that we currently send back the offline template and provide the error message as errorMessages.comment as is returned by wp_service_worker_get_error_messages() and is currently:

Your comment will be submitted once you are back online!

Clearly this would need to be replaced with something like showing the clientOffline or serverOffline message, but then whenever the submission is POST to also include something like this as another paragraph:

Your submission failed. Please go back and try again.

This would probably need to be in bold.

The go back link would be history.back(). With browsers now supporting bfcache, the comment form should still have the user's submission when they go back.

So then the question is whether it should be extended to all POST requests and not just the comment submission from. I suppose?

[rutviksavsani]

So then the question is whether it should be extended to all POST requests and not just the comment submission from. I suppose?

Yes, I guess It should be implemented for all the POST requests.

Also let me know if a seperate issue will be created for the same or should it be implemented in the same one?

[westonruter]

It can be done as part of the same issue, yes.

[pooja-muchandikar]

QA Passed ✅

The expected changes are working fine.

Screen.Recording.2022-04-19.at.7.16.58.PM.mov