1372411 - Newsgroups "Click here to remove all expired articles" should be in chrome UI, not in content

Status: NEW

(MailNews Core :: Networking: NNTP, defect)

Description

[Ben Bucksch (:BenB)]

This is a follow-up of bug 1364723, to fix the UI properly

Steps to reproduce:

I visit a newsgroup which expires articles after 5 days. To reproduce, you must have a known expired article to attempt to open. In my case, I visit the group to get the current list of articles; then return later, >5 days since the posting date of a given article. Open the entry for the expired article.


Actual results:

Since v.52 (to best of my knowledge), when the message tab opens, because it cannot return the expired article, you get this text:
---
Error!
newsgroup server responded:Bad article number

Perhaps the article has expired

<XnsA76E7B2651C9Bmikedmsncom@4.79.142.203> (115502)

Click here to remove all expired articles 

---
The last line is a link, which purges expired articles.


Expected results:

* The text is in the header pane (which has XUL chrome privileges), not the content page (which has the same rights as any poster).
* Alternatively: The link stays in the content pane, but the content has an error UI implemented with a privileged chrome URL, not nntp: or data:
* The link does not say "click here".

Importance:
1. Security
The action changes your news store, so it should only be possible to trigger this from chrome UI, never from news messages. The latter is currently the case. A news poster might construct a message with such a URL as link, with an innocently looking link text. That should not be possible.
2. "Click here":
https://www.w3.org/QA/Tips/noClickHere
https://www.smashingmagazine.com/2012/06/links-should-never-say-click-here/

Comment 1

[Jorg K (CEST = GMT+2)]

(In reply to Ben Bucksch (:BenB) from comment #0)
> * Alternatively: The link stays in the content pane, but the content has an
> error UI implemented with a privileged chrome URL, not nntp: or data:
Indeed, a news: URL is not useful since they need to be loadable.
For the record, the link to clear expired messages is for example:
news://news.mozilla.org:119/mozilla.test.multimedia?list-ids

Ben, can you please elaborate on
  "the content has an error UI implemented with a privileged chrome URL".

Since we surely don't want to introduce a new protocol here, are you suggesting to use a chrome: URL and make sure the content displayed has enough privileged to load it, since usually chrome: URLs don't load from content?

Comment 2

[Ben Bucksch (:BenB)]

What I'm suggesting is:

* Put a "Clear expired articles" button into the header pane, and remove it from the content.

* Only if that turns out not to be viable, then I would suggest, as alternative, to implement the entire error UI in the content pane which says "This message could not load, because it expired" as chrome:// page. Given that messages are normally loaded from chrome://, it should work and the error UI should load with chrome privileges. Now, if you click on a news: link inside another message, and that target produces the error, then it wouldn't work. We might accept that, and just not show the button in this case, or we might decide to use alternative 1. In any case, I recommend against this latter approach. We should leave privileged actions in chrome UI, not in the content pane.

Comment 3

[Richard Marti (:Paenglab)]

Could this be related to bug 492329?

Comment 4

[Jorg K (CEST = GMT+2)]

Looks like it, I'll take a look.

Comment 5

[Jorg K (CEST = GMT+2)]

I've looked at it and refreshed the patch over there in bug 492329. The problem is that the overall logic doesn't change: You place a link in a content page (mockup: attachment 377227 [details]) and a click in content will/should trigger a privileged command.

Bug 492329 only makes a nice content page, the overall concept and mechanism remains the same, see this URL which will be triggered:

function removeExpired() {
  document.location.href = folderUri + "?list-ids";
}

Comment 6

[Jorg K (CEST = GMT+2)]

Actually, when bug 492329 lands real soon now, the "error page" will have a button instead of a link. We can therefore change the mechanism that purges expired articles from the message store by changing the button action and disable the "list-ids" trigger.