Last Comment Bug 795562 - nsNntpIncomingServer allows to subscribe twice to the same newsgroup
: nsNntpIncomingServer allows to subscribe twice to the same newsgroup
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Networking: NNTP (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Thunderbird 19.0
Assigned To: :aceman
:
:
Mentors:
Depends on: 803574
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-29 00:11 PDT by Jens Müller (:tessarakt)
Modified: 2013-02-05 11:48 PST (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (2.74 KB, patch)
2012-10-01 14:36 PDT, :aceman
Pidgeot18: review+
Details | Diff | Splinter Review

Description Jens Müller (:tessarakt) 2012-09-29 00:11:36 PDT
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20100101 Firefox/15.0.1
Build ID: 20120905151427

Steps to reproduce:

When writing a test, I run across the following issue (already posted on mozilla.dev.app.thunderbird):

"When calling server.subscribeToNewsgroup(ng) twice with the same 
newsgroup, it gets subscribed twice. That means that 
server.rootFolder.subFolders contains the same newsgroup twice.

Is that intended behavior?"

Magnus Melin replied (by mail only): "No, sounds like a bug."

So this should be fix, even if there might be no way to trigger this from UI.
Comment 1 :aceman 2012-10-01 00:55:39 PDT
The subscribeToNewsGroup seems to be defined here:
http://hg.mozilla.org/comm-central/file/f7c185914146/mailnews/news/src/nsNntpIncomingServer.cpp#l746

It could be enough to call ContainsNewsgroup (http://hg.mozilla.org/comm-central/file/f7c185914146/mailnews/news/src/nsNntpIncomingServer.cpp#l724) from subscribeToNewsGroup.

There only needs to be decision what return value the function returns if the newsgroup already exists.
It could be an error, but I propose it could also be a success (as the wanted newsgroup is now subscribed as the caller wanted).
Comment 2 :aceman 2012-10-01 14:36:26 PDT
Created attachment 666715 [details] [diff] [review]
patch

Jens, can you try if this patch fixes it for you?
Comment 3 Joshua Cranmer [:jcranmer] 2012-10-11 10:46:56 PDT
Comment on attachment 666715 [details] [diff] [review]
patch

Review of attachment 666715 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry for the delay in reviewing this.
Comment 4 Ryan VanderMeulen [:RyanVM] 2012-10-12 19:23:56 PDT
https://hg.mozilla.org/comm-central/rev/dbdb0f38e089
Comment 5 Jens Müller (:tessarakt) 2012-10-21 12:12:53 PDT
Hi aceman,

sorry, I am currently in the process of moving, and don't have access to my Linux machine with the comm-central checkout at the moment. So I cannot give feedback at the moment.

Jens

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