Page MenuHomePhabricator

Remove ContentHandler::isParserCacheSupported()
Open, Needs TriagePublic

Description

There is no good reason for any kind of content to not support parser cache, so ContentHandler::isParserCacheSupported() seems redundant. If some output really shouldn't be cached, it can be marked as uncacheable using ParserOutput::updateCacheExpiry().

Notable, the handlers for CSS, JS, and JSON currently return false from isParserCacheSupported(). That seems unnecessary.

Event Timeline

One issue here potentially would be that ParserCache when adding a debug message (e.g. <!-- Saved in parser cache blablabla) is wrapping it into HTML comment. JSON/Javascript/CSS will have some trouble with that - HTML comments are not valid CSS/JSON/JS comments, so ParserCache will start breaking pages.

I guess it's wrong for ParserCache (ParserOutput now) to assume that text is HTML text. If we wanted to keep support for these comments, we need to add some knowledge about valid comment wrapping to ContentHandler and delegate actually making the comment string to it in ParserCache.

One issue here potentially would be that ParserCache when adding a debug message (e.g. <!-- Saved in parser cache blablabla) is wrapping it into HTML comment. JSON/Javascript/CSS will have some trouble with that - HTML comments are not valid CSS/JSON/JS comments, so ParserCache will start breaking pages.

But the content of ParserOutput is always HTML, even for JSON/Javascript/CSS pages! We even add fancy styles for syntax highlighting. The raw content doesn't go via the ParserCache.