Page MenuHomePhabricator

Notification state in output HTML is cached excessively
Closed, ResolvedPublic

Description

"You have new messages" seems to be being highlighted in orange whenever you view your talk page. It didn't do this until recently, and this behaviour doesn't make sense as it should be a notification rather than always being highlighted whenever you access the page that the notification was about. If you visit another page then return to the watchlist, then the notification is still present when you view your watchlist.

Event Timeline

Mike_Peel updated the task description. (Show Details)
Mike_Peel raised the priority of this task from to Needs Triage.
Mike_Peel added a subscriber: Mike_Peel.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 23 2015, 11:39 PM
Krenair added a subscriber: Krenair.
Restricted Application added a project: Collaboration-Team-Triage. · View Herald TranscriptAug 24 2015, 3:18 AM

This happens because pages you viewed while you had notifications get cached by your browser, and when your browser checks whether it can reuse the cached version after you've viewed your notifications, we say yes (304).

We can probably solve this by hooking into OutputPage's hook that lets us set Last-Modified to the timestamp of the last change to notification state for logged-in users.

Catrope triaged this task as High priority.Aug 26 2015, 11:38 PM
DannyH set Security to None.
DannyH lowered the priority of this task from High to Normal.Aug 27 2015, 7:17 PM
DannyH added a subscriber: DannyH.
Mattflaschen-WMF renamed this task from "You have new messages" is highlighted in orange whenever you view your talk page to Notification state in output HTML is cached excessively.Sep 16 2015, 11:26 PM
DannyH removed a subscriber: DannyH.Oct 5 2015, 10:54 PM

This happens because pages you viewed while you had notifications get cached by your browser, and when your browser checks whether it can reuse the cached version after you've viewed your notifications, we say yes (304).

We can probably solve this by hooking into OutputPage's hook that lets us set Last-Modified to the timestamp of the last change to notification state for logged-in users.

Hmm, I'm not sure this is right. User::setNewtalk() will call User::invalidateCache(), which calls User::touch(), which is one of the timestamps that is checked by OutputPage::checkLastModified().