1511391 - Cannot focus cursor in the textarea in the sidebar webextension

Status: RESOLVED DUPLICATE of bug 1383910

(WebExtensions :: General, defect, P2)

Description

[Vlad Filippov (:vladikoff)]

Attachment: save-bug.gif

### STR

* Use Firefox 65.0a1 (2018-11-28) (64-bit)
* Install Notes from TestPilot via https://testpilot.firefox.com/experiments/notes/
* Open Welcome note, try to focus the cursor on text

### Expected

Cursor should jump between the textarea lines

### Actual

Nothing happens


### Details

Works fine in 63.0.3 (64-bit)


Might be related to https://bugzilla.mozilla.org/show_bug.cgi?id=1496288

Comment 1

[Vlad Filippov (:vladikoff)]

[Regression window]:
Considering the fact that this issue is not reproducible on latest Firefox Release and Beta builds, using the Mozregression tool I've managed to find the following regression window:

Last good revision: f17b7ba6d0aa737d4f69a0fc3206da8c539225e5
First bad revision: 8e88421b280c2afda62f4ba704ce29701c30549f
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=f17b7ba6d0aa737d4f69a0fc3206da8c539225e5&tochange=8e88421b280c2afda62f4ba704ce29701c30549f

From the pushlog it seems that the Bug 1506547 has caused this regression.

From https://github.com/mozilla/notes/issues/1390

Comment 2

[Emilio Cobos Álvarez [:emilio]]

What editor is this using? Is there any chance to upload to this bug an HTML test-case that reproduces the issue?

Comment 3

[Emilio Cobos Álvarez [:emilio]]

Moving to layout, P2 for now to figure out what's going on and if it's an issue with my patch or with the Notes experiment.

Random other question: Is there a way to inspect the HTML that the extension installs?

Comment 4

[Mark Banner (:standard8)]

From a brief glance at https://github.com/mozilla/notes/blob/master/src/sidebar/index.html, it looks like this is using the "CKEditor".

To inspect/debug, you can go to about:debugging, scroll down to where Firefox Notes is, select "Debug". On the inspector pane of the window that opens, look to the top-right of the toolbar, there's two buttons. Select the left most one (not the ..., the split window like one), select "sidebar/index.html" and that should let you inspect it.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Ok, so the good think is that all the CKEditor demos work on Nightly, so there's definitely something specific to the add-on.

That's great, because I did check CKEditor before landing my patch, since it caused problems in some of the bugs that caused the hacks that my patch removed to creep in our codebase ;)

Anyhow, that's great to know, thanks! I'll try to take a look.

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Ok, so this was fun as hell to find. The reason this bug happens only for the extension is:

https://searchfox.org/mozilla-central/rev/76fe4bb385348d3f45bbebcf69ba8c7283dfcec7/browser/components/extensions/extension.css#16

The low-risk fix is:

https://github.com/mozilla/notes/pull/1431

Now, why in the world is there a -moz-user-select: none rule for the root element for web-extensions? I don't know. :bwinton, maybe you know?

There's a further incompatibility issue here, which is that we don't allow selecting elements with -moz-user-select: none if they're editables... Maybe we should change that.

Comment 7

[Emilio Cobos Álvarez [:emilio]]

I filed bug 1518339 for the interop issue between Blink and WebKit.

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1511391 - Remove problematic -moz-user-select: none on the root.

It prevents any element in the page from getting selected and edited, which is
not really desirable. This is workaround-able by overriding this rule, but seems
a bad default.

If we wanted to make some browser-specific thing not selectable we should target
it more explicitly (like .browser-style > label in this same stylesheet).

Comment 9

[Shane Caraveo (:mixedpuppy)]

I wasn't involved in browser-style when it was created, I assumed this select statement was done for a reason. Removing it will affect all extensions that choose to enable browser_style. I think this should be thought through more.

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Per bug 1383910 comment 7 and the phabricator discussion this is the intended behavior, so I don't think there's a point in keeping this open. My fix to the extension should be merged instead, I guess, and we can fix bug 1518339 too on the layout side.

Comment 11

[Vlad Filippov (:vladikoff)]

This is now fixed in the Notes 4.1.2 release, sorry about the delay!

Comment 12

[Blake Winton (:bwinton) (:☕️)]

And I think this was resolved without my input, so I'm clearing my needinfo. 😉