This is a very easy good first bug for a beginner. The following if declarations are useless as delete foo; doesn't have any effect if foo is null. https://dxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginHost.cpp?q=nsPluginHost.cpp&redirect_type=direct#3138 https://dxr.mozilla.org/mozilla-central/source/dom/plugins/base/nsPluginHost.cpp?q=nsPluginHost.cpp&redirect_type=direct#3153 We should just remove the if

Hi Sylvestre. I'm a beginner and I thought this would be a good experience for me to try and have a crack at this. Please let me know if it's still available!

i'm working on it i'm a beginner too.

I will assign it to the first submitting a patch. Fyi, bug 1334265 is also a good first bug for a beginner

Comment on attachment 8831582 [details] Bug 1334104 - 'if' statement is unnecessary https://reviewboard.mozilla.org/r/108114/#review109192 ::: dom/plugins/base/nsPluginHost.cpp:3151 (Diff revision 1) > version, > (const char* const*)mimetypes, > (const char* const*)mimedescriptions, > (const char* const*)extensions, > mimetypecount, lastmod, fromExtension, true); > - if (heapalloced) > + You should remove this line ::: dom/plugins/base/nsPluginHost.cpp:3152 (Diff revision 1) > (const char* const*)mimetypes, > (const char* const*)mimedescriptions, > (const char* const*)extensions, > mimetypecount, lastmod, fromExtension, true); > - if (heapalloced) > + > delete [] heapalloced; The indentation level is incorrect here

Jason, Muhammed, if you want another good first bug to start with, please send me an email and I will create and assign one for you.

Sylvestre, could you let me know if I properly updated my patch? I see there are now 2 commits. Is there a way I could have just updated my first commit, or is this right?

(In reply to cliffplaysdrums from comment #8) > Sylvestre, could you let me know if I properly updated my patch? I see there > are now 2 commits. Is there a way I could have just updated my first commit, > or is this right? Note: I got some help and used histedit to combine the patches. Hopefully it all turned out okay.

Comment on attachment 8831582 [details] Bug 1334104 - 'if' statement is unnecessary Good, now, you have to find a reviewer for this: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Introduction#Step_4_-_Get_your_code_reviewed

(In reply to Sylvestre Ledru [:sylvestre] from comment #11) > Comment on attachment 8831582 [details] > Bug 1334104 - 'if' statement is unnecessary > > Good, now, you have to find a reviewer for this: > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/ > Introduction#Step_4_-_Get_your_code_reviewed Are you the right person to ask for a review, or is it someone else?

(In reply to Sylvestre Ledru [:sylvestre] from comment #6) > Jason, Muhammed, if you want another good first bug to start with, please > send me an email and I will create and assign one for you. Hi, apologies for the late response if there still is another GFB you could point me to I would like to try tackling it. Thanks again!

(In reply to cliffplaysdrums from comment #12) > Are you the right person to ask for a review, or is it someone else? Should be someone else :)

(In reply to Sylvestre Ledru [:sylvestre] from comment #14) > (In reply to cliffplaysdrums from comment #12) > > Are you the right person to ask for a review, or is it someone else? > Should be someone else :) I see the name "Benjamin" frequently around this area of the code. I'm guessing this could be the triage owner, but I'm having trouble finding out what a triage owner is, and in this case, the triage owner is ":bsmedberg" and not "Benjamin" so I don't know if that's a coincidence or a display setting. How do you suggest finding the right reviewer? The first step *does say* to ask your mentor if it's a mentored bug :)

this is the same person :) Just ask to bsmedberg and you should be fine

Comment on attachment 8831582 [details] Bug 1334104 - 'if' statement is unnecessary https://reviewboard.mozilla.org/r/108114/#review113806 Look good to me! thank you. I'll trigger a try build and then autoland if it works.

NI?myself to keep this on my radar to check try results.

Pushed by bsmedberg@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1fe9a4b7b037 'if' statement is unnecessary r=bsmedberg