Closed
Bug 1159001
Opened 9 years ago
Closed 9 years ago
The find input inside the source editor should select the previous search when being reopened
Categories
(DevTools :: Source Editor, defect, P2)
DevTools
Source Editor
Tracking
(firefox41 fixed)
RESOLVED
FIXED
Firefox 41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: bgrins, Assigned: zer0)
References
(Blocks 1 open bug)
Details
(Whiteboard: [polish-backlog][difficulty=easy])
Attachments
(1 file, 2 obsolete files)
3.22 KB,
patch
|
zer0
:
review+
|
Details | Diff | Splinter Review |
STR: Open up the source editor (for example, the style editor on https://www.mozilla.org/en-US/) Press ctrl+f to find Type "Open". See how results are matched Press Enter to close the find mode Press ctrl+f again Expected: The "Open" text is highlighted to make it easy to type over it and replace that text with something else (as customary for 'find' behavior throughout the browser and other applications) Actual The cursor is put at the end of the string
Reporter | ||
Updated•9 years ago
|
Summary: The find input inside the source editor should select all text when being reopened → The find input inside the source editor should select the previous search when being reopened
Reporter | ||
Updated•9 years ago
|
Whiteboard: [devedition-40][difficulty=easy]
Updated•9 years ago
|
Priority: -- → P2
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → zer0
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 1•9 years ago
|
||
I also took occasion to make the find input in a type="search", with the placeholder instead of the label, that is more consistent with the rest of Firefox UI's search box; making also the input get the whole space available. However, if you prefer the previous setting, I'll revert those changes.
Attachment #8605202 -
Flags: review?(bgrinstead)
Reporter | ||
Comment 2•9 years ago
|
||
Comment on attachment 8605202 [details] [diff] [review] select-search.patch Review of attachment 8605202 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Can you please update the README in the directory to show the updated patched changes needed when importing a new copy of the plugin? Also, I'd like if we had a super basic test in place for this (if nothing else so that it can be expanded upon with things like Bug 1153474). a single browser_editor_search test could suit both of these bugs, I think
Attachment #8605202 -
Flags: review?(bgrinstead) → feedback+
Assignee | ||
Comment 3•9 years ago
|
||
(In reply to Brian Grinstead [:bgrins] from comment #2) > Looks good. Can you please update the README in the directory to show the > updated patched changes needed when importing a new copy of the plugin? I'm not familiar with that; I have just to copy & paste the new diff in the "Localization patches" section I guess, but the diff compared to what, exactly? The same in the patches, or to some previous commit for search.js? > Also, I'd like if we had a super basic test in place for this (if nothing > else so that it can be expanded upon with things like Bug 1153474). a > single browser_editor_search test could suit both of these bugs, I think I'll make one for both then; thanks
Flags: needinfo?(bgrinstead)
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #3) > (In reply to Brian Grinstead [:bgrins] from comment #2) > > > Looks good. Can you please update the README in the directory to show the > > updated patched changes needed when importing a new copy of the plugin? > > I'm not familiar with that; I have just to copy & paste the new diff in the > "Localization patches" section I guess, but the diff compared to what, > exactly? The same in the patches, or to some previous commit for search.js? I believe we will need a diff of the current version with the patch applied and this file: https://github.com/codemirror/CodeMirror/blob/4.2.0/addon/search/search.js. And yes, just needs to get copied/pasted into the patches section.
Flags: needinfo?(bgrinstead)
Assignee | ||
Comment 5•9 years ago
|
||
As mentioned, the test for this behavior will be covered by bug 1153474.
Attachment #8617401 -
Flags: review?(bgrinstead)
Assignee | ||
Updated•9 years ago
|
Attachment #8605202 -
Attachment is obsolete: true
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8617401 [details] [diff] [review] search v2 Review of attachment 8617401 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Please update the commit message to include a little more context. Maybe 'Bug 1159001 - Select text in the Style Editor search input when it is reopened;r=bgrins'
Attachment #8617401 -
Flags: review?(bgrinstead) → review+
Assignee | ||
Comment 7•9 years ago
|
||
Added a better commit message as mentioned.
Attachment #8617401 -
Attachment is obsolete: true
Attachment #8623112 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/23d4c6e36439
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox41:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
Updated•9 years ago
|
Whiteboard: [devedition-40][difficulty=easy] → [polish-backlog][difficulty=easy]
Comment 11•8 years ago
|
||
" Press Enter to close the find mode " There is some mistake in this line. To close 'find' bar is necessary to press " ESC "
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•