New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0005257: profile picture upload glitch #64

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@kawadhiya21

kawadhiya21 commented Apr 15, 2013

I am not sure what should be return type.

@harriswong

This comment has been minimized.

Collaborator

harriswong commented Apr 15, 2013

Hi there,

The return type should be false, based on how the rest of the code handles things in the function. The documentation of this function however says it should be null. What do you think is the best solution here?

Secondly, can you explain why you are removing line93:"if (!in_array($extension, $supported_images)) {"?

Also, is there a reason why you use ""You did not select a file. Please select one because empty"" as your response message? I don't think this message is clear because it will still give me that error if I happen to upload a file without extension,

@kawadhiya21

This comment has been minimized.

kawadhiya21 commented Apr 15, 2013

Okay. I think the best solution is to introduce a new ERROR called FILE_ILLEGAL_EMPTY_FILE_ERROR. It would be really helpful not only in this case but at other places too where the user uploads an empty file.

And I would like you to see line97:"else if (!in_array($extension, $supported_images)) {" So this line93 has been shifted down.

Okay ,to test the last problem, I undid my previous commit and I tried 2 things :

  1. Uploaded a pic without extension. -> Simply Failed
  2. Changed a .js file to .png and tried to upload it -> It uploaded without showing a single error.

Okay , this function mime_content_type returned image/png. I am submitting another patch. Please see that. It covers all the cases. https://github.com/kawadhiya21/ATutor/commit/1ccac3859a260a1be2708030aa62cb5bce2d56d7

@kawadhiya21

This comment has been minimized.

kawadhiya21 commented Apr 15, 2013

Okay. I think the best solution is to introduce a new ERROR called
FILE_ILLEGAL_EMPTY_FILE_ERROR. It would be really helpful not only in this
case but at other places too where the user uploads an empty file.

And I would like you to see line97:"else if (!in_array($extension,
$supported_images)) {" So this line93 has been shifted down.

Okay ,to test the last problem, I undid my previous commit and I tried 2
things :

  1. Uploaded a pic without extension. -> Simply Failed
  2. Changed a .js file to .png and tried to upload it -> It uploaded without
    showing a single error.

Okay , this function mime_content_type returned image/png. I am submitting
another patch. Please see that. It covers all the cases.
kawadhiya21@1ccac38https://github.com/kawadhiya21/ATutor/commit/1ccac3859a260a1be2708030aa62cb5bce2d56d7

On Mon, Apr 15, 2013 at 6:49 PM, Harris Wong notifications@github.comwrote:

Hi there,

The return type should be false, based on how the rest of the code handles
things in the function. The documentation of this function however says it
should be null. What do you think is the best solution here?

Secondly, can you explain why you are removing line93:"if
(!in_array($extension, $supported_images)) {"?

Also, is there a reason why you use ""You did not select a file. Please
select one because empty"" as your response message? I don't think this
message is clear because it will still give me that error if I happen to
upload a file without extension,


Reply to this email directly or view it on GitHubhttps://github.com//pull/64#issuecomment-16384072
.

Kshitij Awadhiya
Metallurgical and Material Science , IIT ROORKEE
email: kawadhiya21@gmail.com
contact : +91-7417729536

@atutor

This comment has been minimized.

Owner

atutor commented Apr 15, 2013

this bug was already assigned to the reporter.

@atutor atutor closed this Apr 15, 2013

@kawadhiya21

This comment has been minimized.

kawadhiya21 commented Apr 15, 2013

So awesome this is. The bug was unassigned even before I solved it. Now you assign it someone else. Infact Harris Wong also ended up replying to me because he was not knowing that it was assigned to someone else much before (in one of the admin's head)

The reason : Because you people don't have a mailing list as you are too busy to maintain one. But you are free enough to maintain an IRC chat where no one replies. So proud of atutor.

I asked to assign the bug to me on irc twice or thrice on which no one replied. I also bet that the other unassigned ones are also assigned to fixed people so that new people just try to solve a bug and they ended up getting trashed.

Thank You

PS: The new patch given by reporter may not handle image files with no extension type specified.

@atutor

This comment has been minimized.

Owner

atutor commented Apr 15, 2013

What do you expect. The person who reported the bug was working on it. And based on what you just wrote, you know it. Don't have a mailing list, you did not look very hard.

You best go find yourself another org to work with. That kind of attitude won't get you anywhere with us.

@kawadhiya21

This comment has been minimized.

kawadhiya21 commented Apr 15, 2013

Sorry for my harsh language.
Yeah when you assign the bug to someone else, the bug tracker should show it. It just started to show now when I submitted the patch for the 3rd time. Its not fair. I even asked to assign it to me on bug tracker. But I got no reply. No one said me that it is already assigned to someone else.

And please keep someone to really reply on irc. I have like asked to assign the bug about 2-3 times but no one even cared to reply. I asked about gsoc project and still no one replied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment