One-click search engines: Don't loose input when leaving keyword input field with click on white list background.

VERIFIED FIXED in Firefox 52

Status

()

Firefox
Preferences
P2
normal
VERIFIED FIXED
9 months ago
2 months ago

People

(Reporter: claas, Assigned: claas)

Tracking

Trunk
Firefox 53
Points:
---

Firefox Tracking Flags

(firefox51 fix-optional, firefox52 verified, firefox53 verified)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

9 months ago
Firefox Desktop's preferences tab "Search" (about:preferences#search) includes a manageable list of "One-click search engines". For each search engine, a keyword can be chosen double-clicking the keyword cell of the engine's row or by alternatively pressing |F2| when the engine's row is selected.

Problem: Changing the keyword and then clicking on the white background of the engine list (below the last engine) looses the input, as if |Esc| had been pressed.

Solution: Persist keyword input when loosing focus by clicking on the white background.
(Assignee)

Comment 1

9 months ago
Looks like we only loose the input when clicking in the "Search Engine" column.
When clicking in the "Keyword" column, the changed keyword is persisted as expected.
It looks like the `if` condition at [0] is the reason.

[0] https://dxr.mozilla.org/mozilla-central/rev/0ba72e8027cfcbcbf3426770ac264a7ade2af090/toolkit/content/widgets/tree.xml#403-404
(Assignee)

Updated

9 months ago
Assignee: nobody → mozilla
Status: NEW → ASSIGNED
Priority: -- → P2
(Assignee)

Comment 2

4 months ago
Created attachment 8818376 [details] [diff] [review]
One-click search engines: Don't loose input when leaving keyword input field with click on white list background
(Assignee)

Updated

4 months ago
status-firefox52: --- → affected
status-firefox53: --- → affected
(Assignee)

Comment 3

4 months ago
Comment on attachment 8818376 [details] [diff] [review]
One-click search engines: Don't loose input when leaving keyword input field with click on white list background

Clicking on the white list background when editing a search engine's keyword triggers a blur event which discarded the input via |gSearchPane.onInputBlur| which used to call |tree.stopEditing(false)|.

I changed the behavior to accept the input unless it was discarded explicitly using the Escape key.
Attachment #8818376 - Flags: review?(jaws)
Comment on attachment 8818376 [details] [diff] [review]
One-click search engines: Don't loose input when leaving keyword input field with click on white list background

Review of attachment 8818376 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks!

Please update your commit message to:
Bug 1292629 - Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used. r=jaws"

Then re-upload the patch here and I will mark it for checkin-needed.
Attachment #8818376 - Flags: review?(jaws) → review+
(Assignee)

Comment 5

4 months ago
Created attachment 8818489 [details] [diff] [review]
Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used
(Assignee)

Updated

4 months ago
Attachment #8818376 - Attachment is obsolete: true
(Assignee)

Comment 6

4 months ago
Comment on attachment 8818489 [details] [diff] [review]
Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used

Thanks for the review, Jared. I have updated the commit message as requested.
Attachment #8818489 - Flags: review?(jaws)
Comment on attachment 8818489 [details] [diff] [review]
Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used

Review of attachment 8818489 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, looks good, and tested well.
Attachment #8818489 - Flags: review?(jaws) → review+
Keywords: checkin-needed

Comment 8

4 months ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/25cf3827a16e
Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used. r=jaws
Keywords: checkin-needed

Comment 9

4 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/25cf3827a16e
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 53
Want to request uplift to aurora for this fix?
Flags: needinfo?(mozilla)
Flags: needinfo?(jaws)
status-firefox51: affected → fix-optional
status-firefox52: affected → fix-optional
Comment on attachment 8818489 [details] [diff] [review]
Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used

Approval Request Comment
[Feature/Bug causing the regression]: introduced with new one-off search implementation, usability issue
[User impact if declined]: it is easy for users to make a change and then have that change get lost if they don't hit "Enter" after typing their keyword
[Is this code covered by automated tests?]: none, manual testing only. it only affects the Search pane of the Preferences
[Has the fix been verified in Nightly?]: manually by me but not by QA
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]:
[String changes made/needed]:
Flags: needinfo?(mozilla)
Flags: needinfo?(jaws)
Attachment #8818489 - Flags: approval-mozilla-aurora?
Comment on attachment 8818489 [details] [diff] [review]
Persist the keyword input field of the one-click search settings when the filed loses focus unless Escape is pressed. Previously the field would only persist if Enter was used

Fix for minor usability regression in preferences pane, let's uplift to aurora for 52.
Attachment #8818489 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/f05b1a13f410
status-firefox52: fix-optional → fixed
I have reproduced this bug with Nightly 51.0a1 (2016-08-05) on WIndows 8.1 ,64 Bit!

This bug's fix is verified with latest Beta and Developer Edition (Aurora) ! 

Build    ID : 20170216105119
User  Agent : Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:52.0) Gecko/20100101 Firefox/52.0

Build    ID : 20170216004023
User  Agent : Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0

[testday-20170217]

Comment 15

2 months ago
I have reproduced this bug with Nightly 51.0a1 (2016-08-05) on ubuntu 16.10,64 Bit!

This bug's fix is verified with latest Beta and Developer Edition (Aurora) ! 

Build    ID : 20170216105119
User  Agent : Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0

Build    ID : 20170216084115
User  Agent : Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0

[testday-20170217]
Thanks!
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
You need to log in before you can comment on or make changes to this bug.