In Bug 1462394, we moved the autocomplete data handling
out of the JsTerm to the Redux store. In the process, we
regress some cases like `await n`, which should display
`navigator`, but isn't anymore when the user types the
whole sequence. Ctrl+Space would still show the popup,
which indicates that the issue is not on the server-side.
This issue is caused because our new code decides that
we should hit the cache when typing the `n`, and there's
nothing in the cache.
Previously, we were clearing the cache as soon as the input
last string wasn't alphanumeric, which we don't anymore.
To fix that, instead of relying on the last string of the
input (which could be wrong in cases like `x.["hello `), we
clear the cache when the autocomplete service returns a null
`matches` property.
In the JsPropertyProvider, we use to return null whenever
there isn't any search done (incorrect input, empty match prop, …).
So it seems like a good idea to bust the cache when the
server returns null.
This requires some changes to the autocomplete service, as well
as some in jsPropertyProvider (e.g. to handle `await `).
Tests are added both on the client and the frontend to make sure
we don't regress this (those tests fail without the actual fix).
Differential Revision: https://phabricator.services.mozilla.com/D13231
--HG--
extra : moz-landing-system : lando
Depends on D13137. I could use help to write the test in a better.
I believe there is a cleaner way to create the front here?
I also had other suggestions for making the fronts more robust in the bug.
Let me know if you think I should try to investigate them more.
Differential Revision: https://phabricator.services.mozilla.com/D13138
--HG--
extra : moz-landing-system : lando
Ideally, it would use TargetFactory. But as that's mochitest chrome,
we don't have natural access to gBrowser/firefox tab.
MozReview-Commit-ID: 4nrfFZu6jAU
Depends on D12734
Differential Revision: https://phabricator.services.mozilla.com/D12735
--HG--
extra : moz-landing-system : lando
For now, we only pass { actor } for all tab target fronts created via DebuggerClient.attachTarget,
whereas parent process target fronts were passing the full form from RootFront.getProcess.
MozReview-Commit-ID: 1H2NxFv8glY
Differential Revision: https://phabricator.services.mozilla.com/D12730
--HG--
extra : moz-landing-system : lando
We were dipatching the "connect" request manually, whereas we should be using
protocol.js specifications. It also help getting rid of another use of "form"
instead of "front"/activeTab in Target class.
MozReview-Commit-ID: IOH4mDprAVL
Depends on D12577
Differential Revision: https://phabricator.services.mozilla.com/D12578
--HG--
extra : moz-landing-system : lando
I did a dedicated patch for converting both about:debuggings as it is slightly more
complex as we have to tweak redux data, wrappers, mocks.
This patch also convert a manual "reload" request being done by about:debugging
and instead use protocol.js front to do it.
Also, I moved isLegacyTemporaryExtension to the front as it depends on accessing the form
and it should be better to keep form processing to the fronts, if possible.
MozReview-Commit-ID: 16qZkuCBp9b
Depends on D12576
Differential Revision: https://phabricator.services.mozilla.com/D12577
--HG--
extra : moz-landing-system : lando
Adapt to the new returned value of listAddons
and also always call it from RootFront instead of DebuggerClient.
Fix the spec in order to expose reload on the front (it was a miss from a previous patch).
MozReview-Commit-ID: AQ5EOQEqnxX
Depends on D12575
Differential Revision: https://phabricator.services.mozilla.com/D12576
--HG--
extra : moz-landing-system : lando
In JsPropertyProvider, if doing a property access (with a dot),
check that the results are suited for a dot notation (e.g. `data`
is, while `data-test` is not).
In case, of an element access, we can return everything.
This implies making some changes to some tests which were using
invalid dot notation access in some case, which revealed a
bug with bracket autocomplete and spaces.
So the bracket autocomplete with spaces is now also fixed, and
a test case was added for that as well.
Differential Revision: https://phabricator.services.mozilla.com/D12726
--HG--
extra : moz-landing-system : lando
We were dipatching the "connect" request manually, whereas we should be using
protocol.js specifications. It also help getting rid of another use of "form"
instead of "front"/activeTab in Target class.
MozReview-Commit-ID: IOH4mDprAVL
Depends on D12577
Differential Revision: https://phabricator.services.mozilla.com/D12578
--HG--
extra : moz-landing-system : lando
I did a dedicated patch for converting both about:debuggings as it is slightly more
complex as we have to tweak redux data, wrappers, mocks.
This patch also convert a manual "reload" request being done by about:debugging
and instead use protocol.js front to do it.
Also, I moved isLegacyTemporaryExtension to the front as it depends on accessing the form
and it should be better to keep form processing to the fronts, if possible.
MozReview-Commit-ID: 16qZkuCBp9b
Depends on D12576
Differential Revision: https://phabricator.services.mozilla.com/D12577
--HG--
extra : moz-landing-system : lando
Adapt to the new returned value of listAddons
and also always call it from RootFront instead of DebuggerClient.
Fix the spec in order to expose reload on the front (it was a miss from a previous patch).
MozReview-Commit-ID: AQ5EOQEqnxX
Depends on D12575
Differential Revision: https://phabricator.services.mozilla.com/D12576
--HG--
extra : moz-landing-system : lando
This is a best effort attempt at ensuring that the adverse impact of
reformatting the entire tree over the comments would be minimal. I've used a
combination of strategies including disabling of formatting, some manual
formatting and some changes to formatting to work around some clang-format
limitations.
Differential Revision: https://phabricator.services.mozilla.com/D13046
--HG--
extra : moz-landing-system : lando
There's a few subtle behavior changes here, which I'll try to break down in the
commit message.
The biggest one is the EditableDescendantCount stuff going away. This
was added in bug 1181130, to prevent clicking on the non-editable div from
selecting the editable div inside. This is problematic for multiple reasons:
* First, I don't think non-editable regions of an editable element should
be user-select: all.
* Second, it just doesn't work in Shadow DOM (the editable descendant count is
not kept up-to-date when not in the uncomposed doc), so nested
contenteditables behave differently inside vs. outside a Shadow Tree.
* Third, I think it's user hostile to just entirely disable selection if you
have a contenteditable descendant as a child of a user-select: all thing.
WebKit behaves like this patch in the following test-case (though not Blink):
https://crisal.io/tmp/user-select-all-contenteditable-descendant.html
Edge doesn't seem to support user-select: all at all (no pun intended).
But we don't allow to select anything at all which looks wrong.
* Fourth, it's not tested at all (which explains how we broke it in Shadow DOM
and not even notice...).
In any case I've verified that this doesn't regress the editor from that bug. If
this regresses anything we can fix it as outlined in the first bullet point
above, which should also make us more compatible with other UAs in that
test-case.
The other change is `all` not overriding everything else. So, something like:
<div style="-webkit-user-select: all">All <div style="-webkit-user-select: none">None</div></div>
Totally ignores the -webkit-user-select: none declaration in Firefox before this
change. This doesn't match any other UA nor the spec, and this patch aligns us
with WebKit / Blink.
This in turn makes us not need -moz-text anymore, whose only purpose was to
avoid this.
This also fixes a variety of bugs uncovered by the previous changes, like the
SetIgnoreUserModify(false) call in editor being completely useless, since
presShell->SetCaretEnabled ended in nsCaret::SetVisible, which overrode it.
This in turn uncovered even more bugs, from bugs in the caret painting code,
like not checking -moz-user-modify on the right frame if you're the last frame
of a line, to even funnier bits where before this patch you show the caret but
can't write at all...
In any case, the new setup I came up with is that when you're editing (the
selection is focused on an editable node) moving the caret forces it to end up
in an editable node, thus jumping over non-editable ones.
This has the nice effect of not completely disabling selection of
-moz-user-select: all elements that have editable descendants (which was a very
ad-hoc hack for bug 1181130, and somewhat broken per the above), and also
not needing the -moz-user-select: all for non-editable bits in contenteditable.css
at all.
This also fixes issues with br-skipping like not being able to insert content in
the following test-case:
<div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>
If you start moving to the left from the second line, for example.
I think this yields way better behavior in all the relevant test-cases from bug
1181130 / bug 1109968 / bug 1132768, shouldn't cause any regression, and the
complexity is significantly reduced in some places.
There's still some other broken bits that this patch doesn't fix, but I'll file
follow-ups for those.
Differential Revision: https://phabricator.services.mozilla.com/D12687
--HG--
extra : moz-landing-system : lando
There's a few subtle behavior changes here, which I'll try to break down in the
commit message.
The biggest one is the EditableDescendantCount stuff going away. This
was added in bug 1181130, to prevent clicking on the non-editable div from
selecting the editable div inside. This is problematic for multiple reasons:
* First, I don't think non-editable regions of an editable element should
be user-select: all.
* Second, it just doesn't work in Shadow DOM (the editable descendant count is
not kept up-to-date when not in the uncomposed doc), so nested
contenteditables behave differently inside vs. outside a Shadow Tree.
* Third, I think it's user hostile to just entirely disable selection if you
have a contenteditable descendant as a child of a user-select: all thing.
WebKit behaves like this patch in the following test-case (though not Blink):
https://crisal.io/tmp/user-select-all-contenteditable-descendant.html
Edge doesn't seem to support user-select: all at all (no pun intended).
But we don't allow to select anything at all which looks wrong.
* Fourth, it's not tested at all (which explains how we broke it in Shadow DOM
and not even notice...).
In any case I've verified that this doesn't regress the editor from that bug. If
this regresses anything we can fix it as outlined in the first bullet point
above, which should also make us more compatible with other UAs in that
test-case.
The other change is `all` not overriding everything else. So, something like:
<div style="-webkit-user-select: all">All <div style="-webkit-user-select: none">None</div></div>
Totally ignores the -webkit-user-select: none declaration in Firefox before this
change. This doesn't match any other UA nor the spec, and this patch aligns us
with WebKit / Blink.
This in turn makes us not need -moz-text anymore, whose only purpose was to
avoid this.
This also fixes a variety of bugs uncovered by the previous changes, like the
SetIgnoreUserModify(false) call in editor being completely useless, since
presShell->SetCaretEnabled ended in nsCaret::SetVisible, which overrode it.
This in turn uncovered even more bugs, from bugs in the caret painting code,
like not checking -moz-user-modify on the right frame if you're the last frame
of a line, to even funnier bits where before this patch you show the caret but
can't write at all...
In any case, the new setup I came up with is that when you're editing (the
selection is focused on an editable node) moving the caret forces it to end up
in an editable node, thus jumping over non-editable ones.
This has the nice effect of not completely disabling selection of
-moz-user-select: all elements that have editable descendants (which was a very
ad-hoc hack for bug 1181130, and somewhat broken per the above), and also
not needing the -moz-user-select: all for non-editable bits in contenteditable.css
at all.
This also fixes issues with br-skipping like not being able to insert content in
the following test-case:
<div contenteditable="true"><span contenteditable="false">xyz </span><br>editable</div>
If you start moving to the left from the second line, for example.
I think this yields way better behavior in all the relevant test-cases from bug
1181130 / bug 1109968 / bug 1132768, shouldn't cause any regression, and the
complexity is significantly reduced in some places.
There's still some other broken bits that this patch doesn't fix, but I'll file
follow-ups for those.
Differential Revision: https://phabricator.services.mozilla.com/D12687
--HG--
extra : moz-landing-system : lando
Without this devtools/server/tests/unit/test_breakpoint-22.js fails in protocol.js
writeError function when trying to use console object.
MozReview-Commit-ID: JFhFboHugUh
Differential Revision: https://phabricator.services.mozilla.com/D12309
--HG--
extra : moz-landing-system : lando
Nicolas Chevobbe
Julian Descottes
Narcis Beleuzu
Alexandre Poirot
Andreea Pavel
Ehsan Akhgari
Daniel Varga
Emilio Cobos Álvarez
Brindusan Cristian
Nazım Can Altınova
yulia
Coroiu Cristina
Mats Palmgren
Michael Ratcliffe
Tom Schuster
Csoregi Natalia