Last Comment Bug 554901 - mailnews.messageid_browser.url is no more valid for mid-searches
: mailnews.messageid_browser.url is no more valid for mid-searches
Status: RESOLVED FIXED
: fixed-seamonkey2.0.7
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Jens Hatlak (:InvisibleSmiley)
:
Mentors:
Depends on:
Blocks: 589593
  Show dependency treegraph
 
Reported: 2010-03-25 04:34 PDT by stefan.blumenrath
Modified: 2010-10-19 10:34 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
.7-fixed


Attachments
patch for all c-c occurrences (2.75 KB, patch)
2010-03-25 11:53 PDT, Jens Hatlak (:InvisibleSmiley)
mnyromyr: review+
mozilla: superreview+
Details | Diff | Splinter Review
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17] (2.74 KB, patch)
2010-03-26 00:14 PDT, Jens Hatlak (:InvisibleSmiley)
jh: review+
jh: superreview+
standard8: approval‑thunderbird3.0.7+
kairo: approval‑seamonkey2.0.7+
Details | Diff | Splinter Review

Description stefan.blumenrath 2010-03-25 04:34:36 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.8) Gecko/20100205 Lightning/1.0b1 Mnenhy/0.8.0pre15 SeaMonkey/2.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.9.1.8) Gecko/20100205 Lightning/1.0b1 Mnenhy/0.8.0pre15 SeaMonkey/2.0.3

The standar value for mailnews.messageid_browser.url is set to http://groups.google.de/groups?selm=%mid&rnum=1
This url isn't valid anymore, it has to be replaced by
http://groups.google.de/groups/search?as_umsgid=%mid&rnum=1

With the given url add-ons (like mnenhy or midfinder) aren't able to find a message by google. 

Reproducible: Always

Steps to Reproduce:
1. use an add-on using this prev for find a message by google
2.
3.
Actual Results:  
Google gives an internal server error, i should retry in 30 seconds.
This happens allways

Expected Results:  
Display the message with the given mid

I use mnenhy for mid-search. in mnenhys source you find:

function OpenBrowserWithMessageId(asMessageID)
{
  let sBrowserURL = goMnenhy.GetPref("mailnews.messageid_browser.url", goMnenhy.kiPrefWString);
...
Comment 1 Tobias Fischer 2010-03-25 05:25:15 PDT
I can reproduce this with:
Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.3a4pre) Gecko/20100324 Mnenhy/0.8.0pre15 SeaMonkey/2.1a1pre

Because I am currently using an Trunk-Build, I will wait until I (or someone else) reproduce this behaviour with SM-2.0.x Branch before I confirm this report.
Comment 2 Robert Kaiser 2010-03-25 05:26:59 PDT
wanted-seamonkey2.0 is useless nowadays, wanted-* flags are only useful for features before a final is shipped. stop using it for stable branches where they are useless and only cause work for triaging the requests.
Comment 3 Jens Hatlak (:InvisibleSmiley) 2010-03-25 11:53:00 PDT
Created attachment 434940 [details] [diff] [review]
patch for all c-c occurrences

Confirming. Karsten, can you please request appropriate TB review(er)s on the patch?
Comment 4 Karsten Düsterloh 2010-03-25 15:34:45 PDT
Comment on attachment 434940 [details] [diff] [review]
patch for all c-c occurrences

>+mailnews.messageid_browser.url=http://groups.google.com/search?as_umsgid=%mid&rnum=1

The parameter &rnum=1 is superfluous.
r=me with that.
Comment 5 Jens Hatlak (:InvisibleSmiley) 2010-03-26 00:14:16 PDT
Created attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

Low risk, only used by OpenBrowserWithMessageId() which doesn't change behavior. Unfortunately I cannot request TB branch approval, maybe the bug needs to be moved for that (but please not in such a way that the SM branch approval request gets lost!).

@KaiRo: Could you drop a short note to localizers that this fix is needed for MessageIDFinder/Mnenhy to work correctly? Thanks.
Comment 6 Jens Hatlak (:InvisibleSmiley) 2010-03-26 00:21:12 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

http://hg.mozilla.org/comm-central/rev/2d4efc6ed8ca
Comment 7 Robert Kaiser 2010-03-26 05:36:23 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

Jens, please send a message to m.d.l10n yourself, as you can explain much better what's up there and why we are somewhat breaking the L10n freeze on the stable branch.
Comment 8 Mark Banner (:standard8) 2010-03-26 07:29:37 PDT
I'm moving this to MailNews Core, as its obvious that is what this affects, and this patch also need thunderbird approval before it can land on branch.
Comment 9 Mark Banner (:standard8) 2010-05-04 05:41:48 PDT
I'm uncomfortable with this patch for 3.0 (and as it has gone into trunk) for several reasons:

1) The string ids haven't been bumped on trunk builds. Not all localisers have changed the strings as a result, and won't have any idea about the change on branches.

2) There is no indication why the url is localised, what happens, what is replaced by what, how to test it. At a minimum, this needs a localisation note adding.

One suggestion was that we don't localise the url. That's a possibility, though KaiRo thinks we should have all non-mozilla urls localised (which seems, to some extent, reasonable, if we truly believe that non-English may have better services available).

3) Even with a follow-up post to .l10n, I'm not convinced that all localisers will pick up and action this change.
Comment 10 Robert Kaiser 2010-05-04 07:24:12 PDT
One possibility would be to go through all affected L10n repos and actually perform the change there, using some script or so. Not sure if we want to do that, though.
Comment 11 Dan Mosedale (:dmose) 2010-05-04 14:39:29 PDT
Agreed that a localization note is needed here.  It strikes me that while it would be good to localize this in the longer term, changing it to non-localized for landing on string-frozen branches would be better than just leaving things broken.  Doing what KaiRo suggests in comment 10 doesn't sound crazy either.
Comment 12 Robert Kaiser 2010-05-05 07:40:45 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

We have cut 2.0.5 now, so moving this to 2.0.6
Comment 13 Jens Hatlak (:InvisibleSmiley) 2010-05-21 14:28:06 PDT
(In reply to comment #9)
> 1) The string ids haven't been bumped on trunk builds. Not all localisers have
> changed the strings as a result, and won't have any idea about the change on
> branches.

The meaning of the string has not changed so there's no need to change the ID. You're correct about the missing notification to localizers of course. That's my fault, I was too reluctant to subscribe to and monitor m.d.l10n just to post one message there. Well, it's too late anyway because now we need to discuss how to proceed first.

> 2) There is no indication why the url is localised, what happens, what is
> replaced by what, how to test it. At a minimum, this needs a localisation note
> adding.

Admitted, I concentrated more on fixing the problem (for en-US) than doing a thorough analysis of the situation--which existed long before I even looked at it. If it helped I'd add a localization note. We'll have to agree on going that way first, though.

> One suggestion was that we don't localise the url. That's a possibility, though
> KaiRo thinks we should have all non-mozilla urls localised (which seems, to
> some extent, reasonable, if we truly believe that non-English may have better
> services available).

<http://mxr.mozilla.org/l10n-central/search?string=mailnews.messageid_browser.url>

Locales that picked up the switch: sv-SE, ko, ru, de, pl, zh-TW, be, it, sk, tr, es-ES [pl looks broken in the MXR search result but is actually OK.]
Locales that missed it: all the rest (several times as many as above).

Locales with non-google.com: nb-NO, de, pl, nn-NO, ca.
Locales with non-google.*: none.

If you ask me, that's just not worth it, also considering possible future changes. If I go to www.google.com with my German SeaMonkey I get a localized version. I guess the same is true for any other locale where a corresponding Google Groups version exists. Let's just stop the in-product l10n of this, shall we?

> 3) Even with a follow-up post to .l10n, I'm not convinced that all localisers
> will pick up and action this change.

So true. (Is it pessimism or just realism? At least I'm not alone here.)
It shouldn't be too hard to mass-change all existing, wrong entries, though. It's really not a technical problem but a matter of sustainability and group dynamics.
Comment 14 Robert Kaiser 2010-07-01 12:55:04 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

Forwarding approval to 2.0.7 as it still stands but 2.0.6 has been cut.
Comment 15 Mark Banner (:standard8) 2010-08-20 08:48:52 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

Ok, I really don't like this, but I'm just going to get it out of the way.

This can land with the proviso that a follow-up bug is filed to a) add localisation notes, and b) change the id, or otherwise resolve getting this updated in *all* localisations for the next trunk releases.
Comment 16 Mark Banner (:standard8) 2010-08-20 08:49:16 PDT
Oh and I still think there should be a note to m.d.l10n when this lands.
Comment 17 Jens Hatlak (:InvisibleSmiley) 2010-08-22 10:23:06 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

http://hg.mozilla.org/releases/comm-1.9.1/rev/73960f7358c1
Comment 18 Jens Hatlak (:InvisibleSmiley) 2010-08-22 11:00:28 PDT
(In reply to comment #16)
> Oh and I still think there should be a note to m.d.l10n when this lands.

<88CdnY3qUuuT-ezRnZ2dnUVZ_vmdnZ2d@mozilla.org>
Comment 19 Jens Hatlak (:InvisibleSmiley) 2010-08-22 12:30:00 PDT
Comment on attachment 435110 [details] [diff] [review]
patch v1a, r=Mnyromyr sr=bienvenu [Checkin: comments 6 and 17]

This also landed on comm-1.9.2 back in March (yes, this bug really is that old!):

http://hg.mozilla.org/releases/comm-1.9.2/rev/2d4efc6ed8ca

(In reply to comment #15)
> This can land with the proviso that a follow-up bug is filed to a) add
> localisation notes, and b) change the id, or otherwise resolve getting this
> updated in *all* localisations for the next trunk releases.

Filed bug 589593.

Note You need to log in before you can comment on or make changes to this bug.