1352776 - In put fields do not blur automatically when selecting a ::before pseudo element in the markup-view

Status: RESOLVED FIXED

(DevTools :: Inspector, defect, P3)

Description

[magicp]

Attachment: editbox-of-boxmodel-does-not-close-automatically.mp4

[Steps to reproduce]
1. Start any versions Firefox
2. Go to about:home
3. Open Inspector > Computed
4. Select #aboutMozilla in markup
5. Click width/height on boxmodel-content > Edit box is enabled
6. Select #aboutMozilla::before in markup

[Actual Results]
Edit box does not close automatically.

[Expected Results]
Edit box closes automatically when switching to other elements.

Comment 1

[Patrick Brosset <:pbro>]

This is a general focus issue, that does not only occur with the width/height fields of the box-model. So I don't think it should block bug 1150496.

You can try this other test case for example:
- select the #aboutMozilla node
- in the rule-view, click inside the "Filter Styles" input
- select the ::before pseudoelement in the markup
--> The focus remains inside the search input.

This does *not* happen with other node types however. If you select a non-pseudoelement in the markup, the focus goes to that element. So this is most probably an issue of the markup-view that does not acquire focus when a pseudo-element is selected.

Also, this happens in older versions of Firefox too. So I don't think it should block bug 1347964 either.

Comment 2

[Patrick Brosset <:pbro>]

Attachment: Bug 1352776 - Make ReadOnlyEditor in the inspector also tabbable like other editors;

Review commit: https://reviewboard.mozilla.org/r/133390/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/133390/

Comment 3

[Patrick Brosset <:pbro>]

Comment on attachment 8861399 [details]
Bug 1352776 - Make ReadOnlyEditor in the inspector also tabbable like other editors;

Hi Yura, I think you had worked on the tabindex logic of the markup-view. Do you mind taking a quick look and telling me if that's OK to do?
I think it should be. I'm essentially just making DOCTYPE and ::before/::after nodes in the markup-view tabbable, so they also acquire focus when selected, just like other nodes.

Comment 4

[Patrick Brosset <:pbro>]

Comment on attachment 8861399 [details]
Bug 1352776 - Make ReadOnlyEditor in the inspector also tabbable like other editors;

Let's get a review in parallel.

Comment 5

[Yura Zenevich [:yzen] (please use "needinfo")]

Comment on attachment 8861399 [details]
Bug 1352776 - Make ReadOnlyEditor in the inspector also tabbable like other editors;

I believe it is, I initially was just duplicating the non-keyboard behavior, so as long as everything is consistent it looks good.

Comment 6

[Julian Descottes [:jdescottes]]

Comment on attachment 8861399 [details]
Bug 1352776 - Make ReadOnlyEditor in the inspector also tabbable like other editors;

https://reviewboard.mozilla.org/r/133390/#review138430

Stealing this quick review :) ! 

LGTM, just a comment nit.

::: devtools/client/inspector/markup/views/read-only-editor.js:27
(Diff revision 1)
>      this.tag.textContent = node.doctypeString;
>    } else {
>      this.tag.textContent = node.nodeName;
>    }
> +
> +  // Make the "tag" part of this editor tabbable so it gets focused when selected.

nit: -1 means the element is focusable but not through keyboard navigation so "tabbable" seems incorrect here.

Comment 7

[Patrick Brosset <:pbro>]

Comment on attachment 8861399 [details]
Bug 1352776 - Make ReadOnlyEditor in the inspector also tabbable like other editors;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/133390/diff/1-2/

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21ea46a299d5
Make ReadOnlyEditor in the inspector also tabbable like other editors; r=jdescottes

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/21ea46a299d5