Closed
Bug 1123431
Opened 10 years ago
Closed 10 years ago
Use case-insensitive search for filtering passwords on about:passwords
Categories
(Firefox for Android Graveyard :: Logins, Passwords and Form Fill, defect)
Tracking
(firefox39 fixed)
RESOLVED
FIXED
Firefox 39
Tracking | Status | |
---|---|---|
firefox39 | --- | fixed |
People
(Reporter: Margaret, Assigned: me, Mentored)
References
Details
(Whiteboard: [lang=js][good first bug])
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
liuche
:
review+
|
Details | Diff | Splinter Review |
Comment 1•10 years ago
|
||
:Margaret i would really like to work on this bug.I m new to open source and i think this can help me to further venture in open source world.
Updated•10 years ago
|
No longer blocks: password-ui
Updated•10 years ago
|
Blocks: mobile-about-passwords
Reporter | ||
Comment 2•10 years ago
|
||
(In reply to vaibhav [:vaibhavChoudhary] from comment #1)
> :Margaret i would really like to work on this bug.I m new to open source and
> i think this can help me to further venture in open source world.
Hi vaibhav! Sorry for the slow response, I was traveling, and I'm just catching up on things now.
First of all, you'll want to set up a Firefox for Android build environment. You can find instructions for that here:
https://wiki.mozilla.org/Mobile/Fennec/Android
Please join us in #mobile on irc.mozilla.org to ask questions if you run into any issues getting a build set up. Also feel free to say hi to introduce yourself!
To fix this bug, you'll want to take a look at the filtering code here:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/aboutPasswords.js#246
To fix this, I think we just need to make the query text lowercase, since we already make the login data lowercase as well.
Feel free to ask Chenxia (liuche on IRC) more questions, since she owns this code :)
Comment 3•10 years ago
|
||
Hi vaibhav! Were you still interested in working on this bug? Someone else asked me about it, and I wanted to make sure you haven't started it already.
Flags: needinfo?(choudharyvaibhav132)
Comment 4•10 years ago
|
||
Margaret no i haven't started working on this bug.I would appreciate if you can pass it to someone else.
Flags: needinfo?(choudharyvaibhav132)
Reporter | ||
Updated•10 years ago
|
Whiteboard: [lang=js] → [lang=js][good first bug]
[Security approval request comment]
How easily could an exploit be constructed based on the patch?
Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?
Which older supported branches are affected by this flaw?
If not all supported branches, which bug introduced the flaw?
Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?
How likely is this patch to cause regressions; how much testing does it need?
Would love if you could take a look at this. I think this is what you were looking for!
Attachment #8561666 -
Flags: sec-approval?
Attachment #8561666 -
Flags: review+
Comment 7•10 years ago
|
||
Comment on attachment 8561666 [details] [diff] [review]
The .diff file for a working patch to this bug.
Clearing sec-approval flag as this isn't a security issue.
Attachment #8561666 -
Flags: sec-approval?
Comment 8•10 years ago
|
||
Comment on attachment 8561666 [details] [diff] [review]
The .diff file for a working patch to this bug.
Thanks for the patch - have you built and verified that this works?
Flags: needinfo?(me)
Attachment #8561666 -
Flags: review+
(In reply to Chenxia Liu [:liuche] from comment #8)
> Comment on attachment 8561666 [details] [diff] [review]
> The .diff file for a potential patch to this bug.
>
> Thanks for the patch - have you built and verified that this works?
Hi, I submitted this patch under supervision at Mozilla offices in Toronto. I wasn't able to built and test the feature, but was told I could forward it to some sort of testing facility. Would love more help on that subject! :)
Flags: needinfo?(me)
Comment 10•10 years ago
|
||
We really expect contributors to at least be able to build and verify their patches locally. One of the main points of these [good first bug]s is to help contributors get their build environment set up by fixing a simple bug, so in the future, they'll have that environment
I think your supervisor at Toronto meant that you didn't need to run *all* the tests locally, but you really do need to be able to build locally. Who is that, by the way? I think I need to have a chat with them :P
Take a look at https://wiki.mozilla.org/Mobile/Fennec/Android for our tutorial on getting set up. Let me know if you run into problems, or drop into #mobile for help!
Assignee | ||
Comment 11•10 years ago
|
||
(In reply to Chenxia Liu [:liuche] from comment #10)
> We really expect contributors to at least be able to build and verify their
> patches locally. One of the main points of these [good first bug]s is to
> help contributors get their build environment set up by fixing a simple bug,
> so in the future, they'll have that environment
>
> I think your supervisor at Toronto meant that you didn't need to run *all*
> the tests locally, but you really do need to be able to build locally. Who
> is that, by the way? I think I need to have a chat with them :P
>
> Take a look at https://wiki.mozilla.org/Mobile/Fennec/Android for our
> tutorial on getting set up. Let me know if you run into problems, or drop
> into #mobile for help!
I won't mention names, but they walked me through the whole process and didn't mention that. Thanks for letting me know, though!
Comment 12•10 years ago
|
||
Hi, i would love to work on this bug. I have set up my build environment. I have a doubt, is my gecko-dev (--depth=1 only) repository need to be up-to-date everyday or i can just work on the bug and submit a patch? if i want to up-to-date my local repository, is there a way to safely merge the changes and build only the parts that changed?
Thanks.
Now i will get going on my first bug.
Flags: needinfo?(nobody)
Comment 13•10 years ago
|
||
Hi, please assign me to this bug. I have tested the patch and its working.
Flags: needinfo?(margaret.leibovic)
Assignee | ||
Comment 14•10 years ago
|
||
(In reply to Bhargav Chippada from comment #13)
> Created attachment 8570023 [details] [diff] [review]
> working patch for using case-insensitive search in about:passwords
>
> Hi, please assign me to this bug. I have tested the patch and its working.
I did too, tested and everything, a couple weeks ago. I don't think they are assigning people to bug and just getting multiple people to complete it to get an idea of how to submit bugs.
Attachment #8561666 -
Attachment description: The .diff file for a potential patch to this bug. → The .diff file for a working patch to this bug.
Comment 15•10 years ago
|
||
Oh no, sorry for the confusion, we do want this bug fixed! I was unaware that qasim, you had ended up building and testing the bug already. I wanted to make sure that you were able to get a build made with the patch applied, but now that you've done that, let me review it. Thanks!
Assignee: nobody → me
Flags: needinfo?(nobody)
Flags: needinfo?(margaret.leibovic)
Updated•10 years ago
|
Attachment #8561666 -
Flags: review+
Comment 16•10 years ago
|
||
I built and tested this, so I'll land it. One thing - the commit message should list the bug number and description and reviewer, e.g.:
Bug 1123431 - Use case-insensitive search for filtering passwords on about:passwords. r=liuche
I'll fix that when I land this though.
Updated•10 years ago
|
Attachment #8570023 -
Attachment is obsolete: true
Comment 17•10 years ago
|
||
Updated•10 years ago
|
Target Milestone: --- → Firefox 39
Comment 18•10 years ago
|
||
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•