Closed
Bug 861478
Opened 11 years ago
Closed 11 years ago
Search field should pre-fill with selected text
Categories
(DevTools :: Debugger, enhancement, P3)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 25
People
(Reporter: jryans, Assigned: dorian)
Details
(Whiteboard: [good first bug][mentor=vporof@mozilla.com][lang=js])
Attachments
(1 file, 1 obsolete file)
2.14 KB,
patch
|
vporof
:
review+
|
Details | Diff | Splinter Review |
If there is text selected in the source view of the debugger and then a search keyboard shortcut is pressed, the search field should start out with the selected text already filled in the field. This is similar to what currently happens with the browser's find in page function.
Updated•11 years ago
|
Priority: -- → P3
Whiteboard: [good first bug][mentor=vporof@mozilla.com][lang=js]
Comment 1•11 years ago
|
||
Hi Victor, I am interested in working on this bug. Can you please help me get started with it? Thanks.
Comment 2•11 years ago
|
||
(In reply to Abhishek Potnis [:abhishekp] from comment #1) Hi, thanks for the interest. You can get the currently selected source editor text via getEditorSelection in debugger-view.js. With that information, you can modify the _doSearch function in the FilterView object from debugger-toolbar.js, which is called whenever a search key is pressed. You'll also need to add some testing for this, and modifying browser_dbg_scripts-searching-01.js with a few more assertions should be enough.
Assignee: nobody → abhishekp.bugzilla
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•11 years ago
|
||
Attachment #766443 -
Flags: review?(vporof)
Comment 4•11 years ago
|
||
Comment on attachment 766443 [details] [diff] [review] added simple fix and unit test Review of attachment 766443 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, this looks good! Thank you Dorian. r+ with a green try run. Do you have try access? If not I can push for you. ::: browser/devtools/debugger/test/browser_dbg_scripts-searching-01.js @@ +266,5 @@ > is(gSources.visibleItems.length, 1, > "Not all the scripts are shown after the search. (20)"); > isnot(gSources.widget.getAttribute("label"), noMatchingSources, > "The scripts container should not display a notice that matches are found."); > + Nit: some extra whitespace here.
Attachment #766443 -
Flags: review?(vporof) → review+
Updated•11 years ago
|
Assignee: abhishekp.bugzilla → dorian
Comment 5•11 years ago
|
||
Try run: https://tbpl.mozilla.org/?tree=Try&rev=56b00727e7ab
Assignee | ||
Comment 6•11 years ago
|
||
Thank you Victor for reviewing my patch so quickly. I don't have try access but have seen that you pushed it.
Comment 7•11 years ago
|
||
(In reply to dorian jaminais from comment #6) > Thank you Victor for reviewing my patch so quickly. > > I don't have try access but have seen that you pushed it. Test seems to be failing on OS X. This is because you'll need to use accelKey instead of ctrlKey when synthesizing the event in the test. OS X uses cmd+F instead of ctrl+F for searching. Oh, and while you're at it, please use "is" instead of "ok" for testing the searchbox value.
Updated•11 years ago
|
Attachment #766443 -
Flags: review+ → feedback+
Assignee | ||
Comment 8•11 years ago
|
||
I applied both recommendation form victor.
Attachment #766443 -
Attachment is obsolete: true
Flags: needinfo?(vporof)
Comment 9•11 years ago
|
||
Comment on attachment 766776 [details] [diff] [review] updated the test to use accelKey instead of ctrlKey Review of attachment 766776 [details] [diff] [review]: ----------------------------------------------------------------- <3
Attachment #766776 -
Flags: review+
Updated•11 years ago
|
Flags: needinfo?(vporof)
Assignee | ||
Comment 10•11 years ago
|
||
:) Thank you very much for your review.
Comment 11•11 years ago
|
||
Pushed again to try: https://tbpl.mozilla.org/?tree=Try&rev=99f95523c250
Comment 12•11 years ago
|
||
There's a timeout on OS X 10.6 debug, but it happens exactly at a sweet spot just before the test finishes. requestingLongerTimeout should be enough, I'll add it just before landing.
Comment 13•11 years ago
|
||
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/35a7fc610f84
Comment 14•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/35a7fc610f84
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 25
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•