Opened 12 years ago

Closed 9 years ago

Last modified 9 years ago

#3207 closed patch (fixed)

Request nickname for authorization request

Reported by: collin Owned by: MarkDoliner
Milestone: 2.7.4 Component: pidgin (gtk)
Version: 2.0.2 Keywords: ICQ SoC
Cc: manski, ivan.komarov

Description

If an ICQ authorization request is received, only the sender's number without nickname was displayed.

Attachments (1)

auth_request_has_no_nick.diff (9.2 KB) - added by collin 12 years ago.

Download all attachments as: .zip

Change History (19)

Changed 12 years ago by collin

comment:1 Changed 12 years ago by lschiere

  • Component changed from unclassified to ICQ
  • Owner changed from lschiere to MarkDoliner

comment:2 Changed 12 years ago by seanegan

  • Milestone set to 2.2.1

comment:3 Changed 12 years ago by Tafkadasom2k5

Maybe it would be good to have a "Get buddyinfo" button here. In some cases it's better to know everything- sometimes I just don't "know" the name, but with the extra-infos I can associate the person.

comment:4 Changed 11 years ago by resiak

  • Component changed from ICQ to pidgin (gtk)
  • Owner MarkDoliner deleted

From #4751, which is closed as a duplicate of this bug:

When someone requests that you let them add you to their buddy list, Pidgin shows a dialog at the bottom of your buddy list asking for confirmation. The confirmation dialog is very brief and does not provide enough information to enable you to make a decision.

Typically, only the screen name of the remote user is shown. However, Pidgin has the user search feature that can usually provide more information.

It would be nice if the request dialog had a button (or something) that would give you easy access to the remote user - maybe even just a link to the search dialog (pre-filled).

Upstreamed from https://bugzilla.novell.com/show_bug.cgi?id=154404 .

comment:5 Changed 11 years ago by pablocpg

i have another trouble, when i send a request to other people using msn, the aren't receving my request, the use msn microsoft. Some body knows what's going on?

comment:6 Changed 10 years ago by rekkanoryo

  • Milestone set to Patches Needing Review

comment:7 Changed 10 years ago by resiak

Some style points:

  • We don't generally add copyright headers to individual files. I see that in some cases there are some copyright headers in two of the changed files, though. In any case, ComBOTS should be added to COPYRIGHT.
  • Code shouldn't be commented out, it should be removed outright. The rationale for removing it lives in the commit message.
  • The brace style is wrong. The rest of oscar.c cuddles the opening brace onto the same line as the if.
  • I really hate bitfielding integers, particularly when it won't make any difference (as I doubt it will for auth_request).

Functional points:

    data->name = g_strdup_printf("%u", info->uin);

Is this ever freed?

    if (info->auth_request)
    {
        g_free(info->reason);
    }

Does this mean that reason might or might not be a freshly-allocated string, depending on auth_request? I'd rather it was consistently dup'd and freed. If instead it's because reason isn't set unless auth_request, g_free(NULL); is a perfectly safe no-op so the if can be removed.

The patch looks like a good idea to me. I appreciate that the patch submitter may have lost interest by now, though. Collin, would you like to update the patch? If not, I guess one of us should do so.

comment:8 Changed 10 years ago by deryni

Ticket #9435 has been marked as a duplicate of this ticket.

comment:9 Changed 9 years ago by rekkanoryo

  • Milestone changed from Patches Needing Review to 2.7.0

This patch needs to be looked at, potentially approved, and somehow accepted before 2.7.0.

comment:10 Changed 9 years ago by rekkanoryo

Erm, I meant "potentially improved".

comment:11 Changed 9 years ago by manski

Note that #5038 seems to be related to this ticket but has additional "features".

comment:12 Changed 9 years ago by rotanid

you are right, but here's already a patch that could be used in 2.7.0

comment:13 Changed 9 years ago by salinasv

  • Milestone changed from 2.7.0 to Patches Needing Review
  • Owner set to MarkDoliner

This patch didn't got into 2.7.0... Asking for a review from Mark

comment:14 Changed 9 years ago by MarkDoliner

  • Cc ivan.komarov added

Hi Ivan. You're of course under no obligation to work on this ticket, but I thought it might be of interest to you. It's related to the ICQ find buddy dialog.

comment:15 Changed 9 years ago by MarkDoliner

Ivan talked to me about this briefly over IM, and I was going to respond to him over IM, but then I figured it was better to leave a comment here, since this is more permanent and Collin might be interested, too.

  • We should avoid adding "--htfv" to most comments. It's sometimes useful in longer comments that describe design decisions or implementation decisions, because it makes it easier for someone to ask the author of the code "why did you do it this way?" But in smaller comments that just annotate the behavior of the code, the "--author" bit isn't really necessary.
  • If we don't think we'll use code again then we should just remove it instead of commenting it out. Otherwise you wind up with lots of commented out code, which makes the source harder to read :-)
  • At the bottom of purple_icqalias() you can just g_free(info->reason). It should be NULL if info->auth_request is FALSE, and g_free(NULL) is harmless.
  • Ivan: I agree that coupling aim_icq_getalias with handling authorization requests makes the code more complicated and may not be the cleanest possible solution, but I'm totally ok with it.

comment:16 Changed 9 years ago by ivan.komarov@…

  • Resolution set to fixed
  • Status changed from new to closed

(In 12ba055ceeb14a00597ed9c99982d8cb3ab6950f):
Show authorization request sender's nickname in the "Authorize buddy?" dialog.

Fixes #3207. I've slightly modified the original patch (courtesy of Collin):

  • the code emitting the "buddy-status-changed" signal was removed, because it doesn't appear to be necessary;
  • "info->uin && info->nick" check in purple_icqalias() was removed, since icqresponse() in family_icq.c already does that.

comment:17 Changed 9 years ago by Robby

  • Keywords ICQ SoC added
  • Milestone Patches Needing Review deleted

I'm adding these keywords a milestone can be assigned more easily whenever the ICQ SoC branch gets merged. :)

comment:18 Changed 9 years ago by MarkDoliner

  • Milestone set to 2.7.4
Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!