Bug 289155

Comment 1 Steffen Pingel 2009-09-10 22:33:45 EDT

That sounds like a serious regression. Frank, can you take a look what could be causing this?

Comment 2 Robert Elves 2009-09-11 13:24:34 EDT

Yes, this is a problem. Investigating.

Comment 3 Robert Elves 2009-09-11 18:50:53 EDT

Created attachment 147012 [details]
work around for bugzilla bug

In 3.4.1 Bugzilla's query page now uses a different identifier for the comment.  They still use the correct standard one in the xml.  This patch will construct the correct url given the repository version and any existing old format queries will be adapted upon execution to the new format if the repo is >= 3.4.1

Comment 4 Robert Elves 2009-09-11 18:52:21 EDT

Frank or Steffen, if you could give this a sanity check that would be great.

Comment 5 Steffen Pingel 2009-09-11 19:03:57 EDT

What would happen if we always sent both parameters in the query url? Just in case Bugzilla decides to "fix" this in the future.

Comment 6 Robert Elves 2009-09-11 20:57:40 EDT

Might just work. I'll do some testing.

Comment 7 Robert Elves 2009-09-11 21:32:05 EDT

Created attachment 147025 [details]
updated patch

Yes. Works fine if both long_desc and longdesc are used in the query for 3.0 and 3.4.1

Comment 8 Steffen Pingel 2009-09-11 23:36:05 EDT

Created attachment 147028 [details]
backported patch

Comment 9 Steffen Pingel 2009-09-11 23:37:25 EDT

I have applied the patch to the 3.2.x branch.

Comment 10 Steffen Pingel 2009-09-12 00:19:44 EDT

Patch committed to 3.2.x branch and head. Thanks for reporting this Benjamin!

Comment 11 Frank Becker 2009-09-12 03:58:58 EDT

Created attachment 147039 [details]
other way for a patch

(In reply to comment #7)
> Created an attachment (id=147025)
> updated patch
> 
> Yes. Works fine if both long_desc and longdesc are used in the query for 3.0
> and 3.4.1

Rob, Steffen

I implement an other way to fix this. Now we only create long_decs for Bugzilla < 3.4 or longdesc for Bugzilla >= 3.4.

This way we can find that code when we remove support for 3.2.

I think we should apply this in HEAD.

Thoughts?

Comment 12 Frank Becker 2009-09-12 03:59:03 EDT

Created attachment 147040 [details]
mylyn/context/zip

Comment 13 Benjamin Muskalla 2009-09-12 06:27:21 EDT

Thanks guys for fixing this that fast - I'm always mesmerized by the Mylyn team ;-)

Comment 14 Steffen Pingel 2009-09-16 13:39:07 EDT

Thanks Frank. That approach is similar to Rob's original patch and I can see how that is more efficient since the search string is only sent ince. The reason we changed it to sending longdesc and long_desc is to handle these cases:

* If the repository configuration is stale the version may not be detected correctly and a search would return wrong results.
* If Bugzilla decides to "fix" this in the future and reverts back to long_desc older Mylyn clients would be broken. 

I'm in favor of keeping the current implementation. I'll reopen this bug for discussion. Rob, has a bug against Bugzilla been filed to clarify why the field was renamed?

Comment 15 Robert Elves 2009-09-16 13:48:23 EDT

  
  
(In reply to comment #14)
> Thanks Frank. That approach is similar to Rob's original patch and I can see
> how that is more efficient since the search string is only sent ince. The
> reason we changed it to sending longdesc and long_desc is to handle these
> cases:
> 
> * If the repository configuration is stale the version may not be detected
> correctly and a search would return wrong results.
> * If Bugzilla decides to "fix" this in the future and reverts back to long_desc
> older Mylyn clients would be broken.
> 
> I'm in favor of keeping the current implementation. I'll reopen this bug for
> discussion. Rob, has a bug against Bugzilla been filed to clarify why the field
> was renamed?


Thanks for investigating this Frank but I think we should stick with the current patch. I've submitted a bug against bugzilla.org for what is essentially an api breakage:

  https://bugzilla.mozilla.org/show_bug.cgi?id=517011

Comment 16 Steffen Pingel 2009-09-16 19:50:41 EDT

Marking resolved then since a work-around is in place on the client side.

Comment 17 Benjamin Muskalla 2009-10-10 19:54:05 EDT

Just stumbled across the code again and saw that you forgot to add the NLS markers for the strings :)

Comment 18 Frank Becker 2009-10-11 03:33:16 EDT

(In reply to comment #17)
> Just stumbled across the code again and saw that you forgot to add the NLS
> markers for the strings :)

I fix the //$NON-NSL- problems in HEAD.

Comment 19 Robert Elves 2009-10-13 19:45:56 EDT

Good catch. Thanks!