inspector-searchbox focus behavior is gone

VERIFIED FIXED in Firefox 50

Status

()

Firefox
Developer Tools: Inspector
P1
normal
VERIFIED FIXED
a year ago
a year ago

People

(Reporter: magicp, Assigned: Honza)

Tracking

(Depends on: 1 bug)

Trunk
Firefox 51
All
Windows
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 verified, firefox51 verified)

Details

(Whiteboard: [reserve-html])

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

a year ago
Created attachment 8780179 [details]
inspector-searchbox-focus-behavior.png

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20160811030201

Steps to reproduce:

1. Start Nightly
2. Open DevTools > Inspector
3. Focus inspector-searchbox


Actual results:

inspector-searchbox focus behavior is gone.


Expected results:

Fix focus-border and focus box shadow.
(Reporter)

Updated

a year ago
Has STR: --- → yes
status-firefox50: --- → unaffected
status-firefox51: --- → affected
Component: Untriaged → Developer Tools: Inspector
OS: Unspecified → All
Hardware: Unspecified → All
(Reporter)

Comment 1

a year ago
Regression range:
https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=c9bbdb627b7804fee47aa6a6708647e6e589d09c&tochange=3269dd1a824d1b42cb021d1fb6858885179940b0
Blocks: 1265759
Has Regression Range: --- → yes
Blocks: 1263741
Flags: qe-verify?
Whiteboard: [devtools-html] [triage]
I can see the blue focus-border on linux(ubuntu 14.04) though, will check on Mac later
(Reporter)

Comment 3

a year ago
(In reply to Fred Lin [:gasolin] from comment #2)
> I can see the blue focus-border on linux(ubuntu 14.04) though, will check on
> Mac later

Hi Fred, you are right. I have changed platform to Windows.
OS: All → Windows
I can see the blue focus-border on Mac as well, so it only effect Windows :-/
Flags: qe-verify? → qe-verify+
Priority: -- → P2
QA Contact: cristian.comorasu
Whiteboard: [devtools-html] [triage] → [reserve-html]

Updated

a year ago
See Also: → bug 1295511

Updated

a year ago
Duplicate of this bug: 1295110
I would assume the same fix here would also apply to Bug 1295511
I borrowed a windows NB and found (by put a `<input/>` tag in jsfiddle)

* xul <textbox> selection have a blue focus-border
* alas normal html <input/> selection does not have a blue focus-border!


On other platform these 2 elements looks like the same. (both have a blue focus-border)



I'd like ask UX team if we'd like unify the blue focus-border effect on `xul <textbox>` and `normal html <input/>` on windows

Morpheus, since you are working on datetime input element, could you help find the right person to decide if we'd like unify or keep html <input> selection effect as it is?
Flags: needinfo?(mochen)
Hi Fred,

As far as I know, the style has been defined in this document below. 

https://firefoxux.github.io/StyleGuide/#/inputs

That means the blue border is the default style, so I'll assume it's a style bug only on Windows. Let me know if any questions or still need a right person to comment, thanks.
Flags: needinfo?(mochen)

Updated

a year ago
Depends on: 1296985
(Assignee)

Comment 9

a year ago
Created attachment 8790609 [details] [diff] [review]
bug1294480.patch

Tim, here is a patch fixing the problem (also fixing it for bug Bug 1295511). There is already a rule for focus, but using -moz-focusring that doesn't work for me (perhaps because we are living in the new non XUL world).

I think we could use this at least till bug 1296985 if fixed.

Honza
Attachment #8790609 - Flags: review?(ntim.bugs)
(Assignee)

Comment 10

a year ago
@ntim: Win64 opt build
https://archive.mozilla.org/pub/firefox/try-builds/jodvarko@mozilla.com-4cfeff7a4dc1cbf19e219f8c253b2b2c16df0769/

Honza
Assignee: nobody → odvarko
Status: NEW → ASSIGNED
Iteration: --- → 51.3 - Sep 19
Priority: P2 → P1

Comment 11

a year ago
Comment on attachment 8790609 [details] [diff] [review]
bug1294480.patch

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

::: devtools/client/themes/common.css
@@ +586,5 @@
>    margin-left: 0;
>    margin-right: 0;
>  }
>  
> +/* Might be removed when bug 1296985 is fixed. */

I've checked the bug in question, and I think it might be invalid. Windows native input styling doesn't have a blue glow.

@@ +589,5 @@
>  
> +/* Might be removed when bug 1296985 is fixed. */
> +.devtools-searchbox > .devtools-textinput:focus,
> +.devtools-searchbox > .devtools-searchinput:focus,
> +.devtools-searchbox > .devtools-filterinput:focus {

I'd go further and remove .devtools-searchbox >, so it works in the memory tool and the DOM panel as well.
Attachment #8790609 - Flags: review?(ntim.bugs)
(Assignee)

Comment 12

a year ago
Created attachment 8791126 [details] [diff] [review]
bug1294480.patch

(In reply to Tim Nguyen :ntim (use needinfo?) from comment #11)
> Comment on attachment 8790609 [details] [diff] [review]
> bug1294480.patch
> 
> Review of attachment 8790609 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/themes/common.css
> @@ +586,5 @@
> >    margin-left: 0;
> >    margin-right: 0;
> >  }
> >  
> > +/* Might be removed when bug 1296985 is fixed. */
> 
> I've checked the bug in question, and I think it might be invalid. Windows
> native input styling doesn't have a blue glow.
Removed

> 
> @@ +589,5 @@
> >  
> > +/* Might be removed when bug 1296985 is fixed. */
> > +.devtools-searchbox > .devtools-textinput:focus,
> > +.devtools-searchbox > .devtools-searchinput:focus,
> > +.devtools-searchbox > .devtools-filterinput:focus {
> 
> I'd go further and remove .devtools-searchbox >, so it works in the memory
> tool and the DOM panel as well.
Done 

Thanks!

Honza
Attachment #8790609 - Attachment is obsolete: true
Attachment #8791126 - Flags: review?(ntim.bugs)

Updated

a year ago
Attachment #8791126 - Flags: review?(ntim.bugs) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 13

a year ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/38fceae2b551
inspector-searchbox focus behavior is gone; r=ntim
Keywords: checkin-needed

Comment 14

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/38fceae2b551
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox51: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
(Reporter)

Comment 15

a year ago
verified in 51 (Build ID 20160916030204)

Comment 11
>I'd go further and remove .devtools-searchbox >, so it works in the memory tool and the DOM panel
>as well.

Can we uplift this to 50 ?
status-firefox51: fixed → verified
(Assignee)

Comment 16

a year ago
Comment on attachment 8791126 [details] [diff] [review]
bug1294480.patch

Approval Request Comment
[Feature/regressing bug #]: 1265759
[User impact if declined]: Missing focus border in a search field (DevTools Toolbox)
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: Low risk, just css change
[String/UUID change made/needed]: n/a
Attachment #8791126 - Flags: approval-mozilla-aurora?
Hi :Honza,
50 is beta now. You should uplift to 50 beta.
Flags: needinfo?(odvarko)
Comment on attachment 8791126 [details] [diff] [review]
bug1294480.patch

It's already in 51.
Attachment #8791126 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
(Assignee)

Comment 19

a year ago
Comment on attachment 8791126 [details] [diff] [review]
bug1294480.patch

Approval Request Comment
[Feature/regressing bug #]: 1265759
[User impact if declined]: Missing focus border in a search field (DevTools Toolbox)
[Describe test coverage new/current, TreeHerder]: n/a
[Risks and why]: Low risk, just css change
[String/UUID change made/needed]: n/a
Flags: needinfo?(odvarko)
Attachment #8791126 - Flags: approval-mozilla-beta?
Hi Honza, Fx50 status is marked as unaffected. Is that incorrect?
Flags: needinfo?(odvarko)

Comment 21

a year ago
(In reply to Ritu Kothari (:ritu) from comment #20)
> Hi Honza, Fx50 status is marked as unaffected. Is that incorrect?

This fixes a bug that affects 50 as well (see comment #15)
Flags: needinfo?(odvarko)
(In reply to Tim Nguyen :ntim (use needinfo?) from comment #21)
> (In reply to Ritu Kothari (:ritu) from comment #20)
> > Hi Honza, Fx50 status is marked as unaffected. Is that incorrect?
> 
> This fixes a bug that affects 50 as well (see comment #15)

Gotcha.
status-firefox50: unaffected → affected
Comment on attachment 8791126 [details] [diff] [review]
bug1294480.patch

CSS only, low risk fix, Beta50+
Attachment #8791126 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 24

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/711dc31cbbad
status-firefox50: affected → fixed
I reproduced this issue using Fx 51.0a1, Build ID: 20160914030200.
I can confirm this issue is fixed, I tested it using Fx 50.0b1, build ID: 20160920155715, on Windows 10 x64, Mac OS X 10.11 and Ubuntu 14.04 LTS.
Status: RESOLVED → VERIFIED
status-firefox50: fixed → verified
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.