664521 - Correct URL for update check link in about:plugins

Status: RESOLVED WORKSFORME

(Toolkit :: General, defect)

Description

[Nochum Sossonko [:Natch]]

Attachment: patch

The URL for the link in about:plugins is mozilla.com/plugincheck missing the locale (e.g. en-US) part.

Attached is a patch for this.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Why is this a problem? mozilla.com does locale auto-detection, so this should be working fine.

Comment 2

[Nochum Sossonko [:Natch]]

(In reply to comment #1)
> Why is this a problem? mozilla.com does locale auto-detection, so this
> should be working fine.

It does work, this just aligns the url with the common preffed url. I don't think this is highly important, although I can't speak of the differences between inserting %LOCALE% in a url or just allowing the website to redirect as it deems fit.

E.g. what if someone manually changes the locale, does mozilla.com detect that?

Comment 3

[Nochum Sossonko [:Natch]]

Also, this will help if an addon changes the preffed url to its own plugin check page.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

Axel may be able to comment on what we should prefer (explicit locale insertion vs. relying on the redirect), but AFAIK we currently do both, somewhat inconsistently.

Comment 5

[Nochum Sossonko [:Natch]]

Attachment: patch

Messed up the switch, this version fixes that.

Built and tested.

Comment 6

[Axel Hecht [:Pike]]

I think we should do locale detection based on accept-lang, and no hard-coded locale code in the url.

Comment 7

[Nochum Sossonko [:Natch]]

Attachment: patch

This should do then. Fixes it in all locations...

Comment 8

[Nochum Sossonko [:Natch]]

Michael, I think this bug may interest you (unless I'm cc'ing the wrong person). Do you do mozilla website stuff?

Comment 9

[Axel Hecht [:Pike]]

Comment on attachment 539831 [details] [diff] [review]
patch

Looks OK, I'm explicitly not doing a review, though.

Comment 10

[Michael Wu [:mwu]]

(In reply to comment #8)
> Do you do mozilla website stuff?

Nope.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 539831 [details] [diff] [review]
patch

You could put the Services.jsm import at the top of the file and then use  Services.strings for the string bundles.

If you're going to depend on a product-specific pref, you probably should do a better job of handling it not existing, e.g. by not showing that link (formatURLPref returns "about:blank" in those cases). I suppose other users of that pref don't do this properly, but they probably should.

Comment 12

[Nochum Sossonko [:Natch]]

(In reply to comment #10)
> (In reply to comment #8)
> > Do you do mozilla website stuff?
> 
> Nope.

Don't know who does then...

New patch coming soon!

Comment 13

[Nochum Sossonko [:Natch]]

Attachment: patch v2

Review comments addressed.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 543850 [details] [diff] [review]
patch v2

This dissociates the link value from the link label, which doesn't seem ideal. I think we probably should just WONTFIX this and leaves things hardcoded in two places (but just change the pref so that they use the same URL).

Comment 15

[Axel Hecht [:Pike]]

Can we generate the label programmatically instead?

I don't feel too great about the other two links, too, but that's probably outside the scope of this bug.

Comment 16

[Nochum Sossonko [:Natch]]

I'm not sure I'm following the issue here, is it that we're removing the link value from the .properties file (hence "disassociating" it from the link label)? By "generate the label programmatically" do you mean to take a substring of the url as the label?

I'm open to whatever is desired here, if WONTFIX is the ultimate decision, that's fine too.

Comment 17

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #16)
> I'm not sure I'm following the issue here, is it that we're removing the
> link value from the .properties file (hence "disassociating" it from the
> link label)?

Yes - someone may not realize that changing one involves changing the other.

If you want to fix the broader issue, we could move all of the URLs to prefs, and remove the string labels and generate the label programmatically based on the URL, as Axel suggests. But I'm not sure it's really worth the trouble.

Comment 18

[Nochum Sossonko [:Natch]]

(In reply to Gavin Sharp from comment #17)
Ok, reassigning to default.

Comment 19

[Virtual_ManPL [:Virtual] 🇵🇱 - (please needinfo? me - so I will see your comment/reply/question/etc.)]

This bug can probably be marked as INVALID per bug #845022

Comment 20

[Axel Hecht [:Pike]]

about:plugins and about:addons still link to different urls for teh plugin checker.

Comment 21

[Mark Banner (:standard8)]

I can't find that URL in the current base, and about:plugins seems to point to support now (if I read correctly). I think we can close this.