With previous change, KeyboardEvent is dispatched even when invisible window
has focus. However, nsRootWindow::GetControllerForCommand() returns controller
for focused window even when the window is invisible because it uses
nsFocusManager::GetFocusedDescendant() to retrieve focused window.
Perhaps, we can assume that users won't expect to do something with invisible
window when they type some keys. Then, nsRootWindow::GetControllerForCommand()
should return controller for visible ancestor window if focused window is
invisible.
This patch makes nsFocusManager::GetFocusedDescendant() can return only visible
descendants. However, it already has a bool argument. Therefore, it should
have a flag instead of adding new flag. Most changes of this patch is replacing
its callers.
Then, nsRootWindow::GetControllerForCommand() and nsRootWindow::GetControllers()
should have a bool flag if it should return controller(s) for visible window.
This patch adds a bool flag for it. Fortunately, the interface isn't scriptable.
Finally, this patch makes nsXBLPrototypeHandler::DispatchXBLCommand() and
EventStateManager::DoContentCommandEvent() retrieve controller for visible
window since they are always handles user input.
MozReview-Commit-ID: GygttTHuKRm
--HG--
extra : rebase_source : 1341273c4606298cb9b890b9312d9f5c8a75d144
In a number of places, there's no substantial use of maps any more
after the segue.
The ELEMENT segue tries the FragmentOrElement QI, but that is
redundant with the Element QI.
This lets me use a few higher-level macros.
MozReview-Commit-ID: Gstq3Cm8LDl
--HG--
extra : rebase_source : f0c7dbf5281ce7375b1369b49db095a211569d6c
The motivation of this patch is that clearing mPresContext should be an
implementation detail of XBL style set, so I create a method for that, and
remove ClearPresContext().
MozReview-Commit-ID: Ftta0rcAqu6
--HG--
extra : rebase_source : 0bdb1ac3e2c988e5a0220172f19ab4b006e883d7
This fixed layout/style/test/test_media_queries_dynamic.html after Part 3 is added.
MozReview-Commit-ID: 7ZpSunCnkIc
--HG--
extra : rebase_source : 6acab11ba784b801b90afc0558c32baabcc3f271
The method always returns NS_OK, and no other caller checks the nsresult.
Hence the patch.
MozReview-Commit-ID: CnYCZ8VchG
--HG--
extra : rebase_source : 8626332e2774c1d6f42c7afa5e9679091a3aeaa2
The method always returns NS_OK, and no other caller checks the nsresult.
Hence the patch.
MozReview-Commit-ID: CnYCZ8VchG
--HG--
extra : rebase_source : 062d643308b309dfe442384301860ffeec2dff9f
This is straightforward, with only two notable things.
- `#include "nsXPIDLString.h" is replaced with `#include "nsString.h"`
throughout, because all nsXPIDLString.h did was include nsString.h. The
exception is for files which already include nsString.h, in which case the
patch just removes the nsXPIDLString.h inclusion.
- The patch removes the |xpidl_string| gtest, but improves the |voided| test to
cover some of its ground, e.g. testing Adopt(nullptr).
--HG--
extra : rebase_source : 452cc4a08046a1adb1a8099a7e85a1917de5add8
We should not be declaring forward declarations for nsString classes directly,
instead we should use nsStringFwd.h. This will make changing the underlying
types easier.
--HG--
extra : rebase_source : b2c7554e8632f078167ff2f609392e63a136c299
There's only one case of sync frame construction from ContentRemoved now, and
it's not on the element being removed, but on the whitespace siblings if needed,
and _only_ when they don't support lazy frame construction.
Basically, this switches all the RecreateFramesForContent calls to use
`aAsyncInsert` (which I changed to an enum class for readability), except when
we're already reframing.
Also, it switches ReframeTextIfNeeded to opt-in into lazy frame construction,
since it's used only when aFlags == CONTENT_REMOVED.
This allows to simplify the DestroyFramesFor API (which I'm happy to rename to
something more meaningful, since now it's something like
DestroyFramesForAndRecreateThemAsync), and do some other consistency cleanups.
A bunch of the ContentRemoved callsites were pretty random at passing
aAsyncInsert, and that was some kind of a mess. This patch ensures consistency,
and makes it impossible to do O(n^2) work when removing DOM nodes, which is
nice.
The underlying reason for this is explained in the description of bug 1377848,
and basically allows us to remove a bunch of Servo hacks on the longer term (a
few of them are going away already, yay!).
MozReview-Commit-ID: 2DrUTxGV8RX
--HG--
extra : rebase_source : f428d839a5482477dea22c0fea600d54f3e8799c
Our current machinery for enabling stylo requires a docshell - if there isn't
one, we default to the Gecko style system.
When getComputedStyle operates on an element without a presshell, it uses the
caller's presshell instead. If the element has previously been styled with
one style system (but no longer has a presshell), and the caller uses a
different style backend, using the caller's style system can cause crashes when
we pull bits of cached data off the DOM (like cached style attributes).
So we want to throw when window.getComputedStyle(element) is called for a
(window, element) pair with different style backends (which is what the next
patch in this bug does).
However, that causes a few failures where stylo-backed documents try to do
getComputedStyle on an XHR document (which, without a docshell, will use the
gecko style system).
So this patch does some work to propagate the creator's style backend into
various docshell-less documents. This should allow both chrome (which uses gecko)
and content (which uses stylo) to use getComputedStyle on the response document
for XHRs they create.
Note that the second patch in this bug will make
chromeWin.getComputedStyle(contentObj) throw. If we discover code that does
that, we can just make it invoke the content's getComputedStyle method over Xrays.
MozReview-Commit-ID: 5OsmHJKq5Ui
Editor treated in nsXBLWindowKeyHandler is always HTMLEditor. So, it should treat the editor as is.
MozReview-Commit-ID: 7uE2p8t5cNW
--HG--
extra : rebase_source : 66b2f42d9d0281f4a244f9cd1ceb077e32ac3515
Before if a key input was handled with APZ we wouldn't dispatch it to content,
because it was guaranteed there wouldn't be any key listeners. Now we are
allowing passive key listeners behind a pref, and so we will sometimes handle
a key input in APZ and dispatch it to content. When this happens, XBL shouldn't
do the default action, otherwise we would double scroll.
MozReview-Commit-ID: 2zfszQ3itkp
--HG--
extra : rebase_source : 724b110fd55c5935df589f1cbcc0b3380028a1ed
This patch replaces four functions of the name AssignWithConversion which
are essentially wrappers around CopyASCIItoUTF16 and LossyCopyUTF16toASCII
with direct calls to the latter two functions. The replaced functions are:
void nsCString::AssignWithConversion( const nsAString& aData )
void nsString::AssignWithConversion( const nsACString& aData )
void nsTString_CharT::AssignWithConversion(
const incompatible_char_type* aData,
int32_t aLength = -1);
The last of the three exists inside the double-included nsTString* world and
so describes two functions, giving four in total.
This has two advantages:
* it removes code
* at the call points, it makes clear (from the replacement name) which
conversion is being carried out. The generic name "AssignWithConversion"
doesn't make that obvious -- one had to infer it from the types.
The patch also removes two commented out lines from
editor/composer/nsComposerCommands.cpp, that appear to be related. They are
at top level, where they would never have compiled. They look like
leftovers from some previous change.
--HG--
extra : rebase_source : fb47bf450771c3c9ee3341dd14520f5da69ec4f5
nsMenuBarListener::KeyPress() is eKeyEvent listener in the system event group. If the target is a remote process, it shouldn't handle accesskey immediately because preceding eKeyDown event may be consumed in the remote process or eKeyPress event itself may be consumed in the remote process.
This patch makes nsMenuBarListener::KeyPress() mark eKeyPress event as "waiting reply from remote process" only when the event matches with a menu item's accesskey and it will be send to a remote process later. Then, reply event should be handled in this method if it's available.
MozReview-Commit-ID: KOpCVgElnca
--HG--
extra : rebase_source : 881ec01f5c8e21c790bf9a8c3167d6c3f932524a
The issue here is that the DestroyFramesFor call does a ClearServoDataFromSubtree,
and then we subsequently fail to load the bindings, leaving ourselves with an unstyled
subtree that isn't marked with descendant bits and isn't rooted at a display:none element,
which is forbidden. This can trigger crashes when we call the innerText getter on one of
the unstyled elements, since that will first flush layout (which won't find the unstyled
subtree), and then invoke IsOrHasAncestorWithDisplayNone, which passes LazyComputeBehavior::Assert.
MozReview-Commit-ID: 7roBkH9fTGZ
The main reason to not do this would be performance (avoiding the
addref/release), but there are two main mitigating factors:
1) All calls to UnwrapReflectorToISupports that pass in a Web IDL object
already do the addref (and in fact QI). So this only affects the
XPCWrappedNative case.
2) The vast majority of the callers proceed to QI on the pointer anyway, and a
second addref is cheap; it's the first addref after a CC that can be
expensive on a cycle-collected object.
Going through the changes one by one:
* In GlobalObject::GetAsSupports, we do have a change that slightly slows down
precisely in the XPCWrappedNative global case. That's the message managers
and the backstagepass. And this really only affects calls to Web IDL statics
from those globals.
* In UnwrapArgImpl we're talking about a Web IDL method taking an "external
interface" type, and the UnwrapReflectorToISupports call is immediately
followed by QI anyway.
* In UnwrapXPConnectImpl we're talking about the case when we have a
non-WebIDL-object implementation of a Web IDL interface. Again, this is the
message manager globals, for EventTarget. And we have a QI call immediately
after the UnwrapReflectorToISupports.
* In the generated HasInstance hook for EventTarget we will be slightly slower
when the LHS of the instanceof is an XPCWrappedNative. And not much slower,
because again there's an immediate QI.
* In InstallXBLField we're never going to have an XPCWrappedNative as thisObj;
it's always an Element in practice. So this is no more expensive than before.
* In sandbox's GetPrincipalOrSOP we now have an extra addref. But it was
followed by various QIs anyway.
* In XPCConvert::JSValToXPCException we have an extra addref if someone throws
an XPCWrappedNative, which is fairly unlikely; our actual Exception objects
are on Web IDL bindings. Plus we have an immediate QI.
* In xpc::HasInstance we have an extra addred if the LHS of instanceof is an
XPCWrappedNative. But, again, there's an immediated QI after the
UnwrapReflectorToISupports.
* In xpcJSWeakReference::Init we are likely doing an extra addref, but again
immediately followed by QI.
I think it's worth making this change just to remove the footgun and that the
perf impact, if any, is pretty minimal.
Most of these changes are just replacements of GetNativeOfWrapper with
UnwrapReflectorToISupports, which is all it did under the hood.
The other changes are as follows:
* In nsDOMClassInfo, we really care whether we have a window, so we can just
UNWRAP_OBJECT to the Window interface, since Window is always on Web IDL
bindings now. Also, the weird compartment check hasn't been needed ever since
GetNativeOfWrapper stopped returning things off the passed-in object's
prototype chain (Firefox 22, bug 658909).
* The only use of do_QueryWrapper was to get a Window in nsDocument; again we
can UNWRAP_OBJECT.
* In XPCJSRuntime, we again just want to check for a Window, so UNWRAP_OBJECT.
Currently, we have 2 bool flags (and optional 2 bool flags with related purpose) for managing propagation state between parent process and remote process. However, it's really complicated. Actually, setting these flags and referring the flags is usually follow explanation.
So, for making simpler, WidgetEvent and BaseEventFlags should have some utility methods for making them as self documented code.
This patch moves WidgetKeyboardEvent::mIsReserved to BaseEventFlags::mIsReservedByChrome. That allows us to manage the cross process event propagation state in same place.
MozReview-Commit-ID: IXEDQJ4GpAZ
--HG--
extra : rebase_source : 5b63ac4f1d15e40e8bfc88423e336de28caa8ab6
We need to call nsBindingManager's method from ServoStyleSet in a later
patch.
Also, we call nsStyleSet::SetBindingManager() immediately after we call
nsStyleSet::Init(). Therefore, we could pass binding manager to Init()
directly, and delete SetBindingManager().
MozReview-Commit-ID: 43WTpW3mSKX
--HG--
extra : rebase_source : f24f6085c51dfae1a969aee1e073720097d240d5
This is a better name for the header that matches its main class.
MozReview-Commit-ID: KSt9LVT3yRR
--HG--
rename : gfx/layers/apz/src/Keyboard.cpp => gfx/layers/apz/src/KeyboardMap.cpp
rename : gfx/layers/apz/src/Keyboard.h => gfx/layers/apz/src/KeyboardMap.h
extra : rebase_source : ec4c9ec5afa8479dc4cd3e987fc3293f047beb9e
extra : histedit_source : caa3a11043fc1ccdff3d7853eda69a0a3dfff99e
This commit makes it so we initialize, send, and store a KeyboardMap for every
APZCTreeManager for later keyboard event processing.
This is a naive approach so it may be worth improving.
MozReview-Commit-ID: CYTbLL3wRlC
--HG--
extra : rebase_source : 016b80a2a209d4fb5b92bc3adb95bd2dbb4399e0
The XBL bindings used for scrolling are managed by a nsXBLWindowKeyHandler. This
class loads the handlers and has logic for searching through them to match a
keyboard event. This commit adds a KeyboardMap class for storing KeyboardShortcuts
and for mirroring the search logic of nsXBLWindowKeyHandler.
MozReview-Commit-ID: 5BmFBilKTJy
--HG--
extra : rebase_source : 96cd0f5808ba6c4926e9da368ab3780b1d9d1449
Keyboard scrolling works by first dispatching a key event to the focused element
of the page. It is then caught by a XBL binding put on the chrome event handler of
every window. The XBL binding searches through all of its handlers to find one
that can handle the keyboard event. The matching binding has a command string
which is dispatched to the nsGlobalWindowCommands which dispatches to PresShell
which does the actual scrolling.
To do this asynchronously, we need a representation of the XBL handlers that can
be applied to a KeyboardInput to get a KeyboardAction.
This commit adds KeyboardShortcut for this purpose. KeyboardShortcut is designed
to be compatible with nsXBLPrototypeHandler and to only handle the specific cases
we care about for keyboard scrolling. If a XBL handler runs javascript or does
anything else we cannot handle in an OMT situation, then we create a
dispatch-to-content KeyboardShortcut.
MozReview-Commit-ID: 1qzywS3QHVp
--HG--
extra : rebase_source : 8ea4f85c02d04ce5e0fa6e430c44ac920269dd9f
All the instances are converted as follows.
- nsAFlatString --> nsString
- nsAFlatCString --> nsCString
--HG--
extra : rebase_source : b37350642c58a85a08363df2e7c610873faa6e41
All the instances are converted as follows.
- nsASingleFragmentString --> nsAString
- nsASingleFragmentCString --> nsACString
--HG--
extra : rebase_source : e6a1ddc8938fecd9a735b15e872c054edf4c7910
This is necessary to facilitate the transition to cloning attributes instead of reparsing them.
MozReview-Commit-ID: Gyd1tD6ldly
--HG--
extra : rebase_source : 777cfed750c95c448f953a6ec98026481997e227
Disable 461917-1.xhtml under dom/xul/crashtests/ because I cannot reproduce
it locally.
Annotate assert-if on some crashtests (bug 1370830). Other modifications are
all removal of fails-if.
MozReview-Commit-ID: 6Q2A5M5rOry
--HG--
extra : rebase_source : b282a919fcda6287fe2eaa6b139a11f2663ad578
In gecko style backend, nsXBLPrototypeResources calls GatherRuleProcessor()
after all the XBL stylesheets are loaded. We use ServoStyleSet to implement
the similar functionality for stylo.
MozReview-Commit-ID: 431XGFALtDY
--HG--
extra : rebase_source : 34327d71e1c88717463b2e8f7dfbda804aca0664
We'll need the bound element to get PresContext in a later patch.
MozReview-Commit-ID: 2Gcb0yKgbTH
--HG--
extra : rebase_source : 6549eb726c32e25bea6045027805492984521d48
The <content> in an XBL document could be injected into documents with
different style backend types. Therefore, we need to set the XBL document's
style backend to the same as the bound document's so that the style
attribute of the content can be processed by the correct backend. <marquee>
elements in xbl-marquee.xml is one such example.
MozReview-Commit-ID: 7M33zlbZqNF
--HG--
extra : rebase_source : 363fd7c2bc91e959fbcfb288bdc1b65cfd1bba97
We assume those XBL documents without a bound document use gecko style
backend. Their backend type will be explicitly set when loading in Part 6.
MozReview-Commit-ID: 5c5xtYFvgAD
--HG--
extra : rebase_source : 578b50953d98ffa5280dac3440979e2aba7519cb
Currently, attribute and child arrays (implemented in dom/base/nsAttrAndChildArray.h) start out empty. When cloning, the array ends up being resized multiple times in order to add the attributes and children that are being cloned from the original node. This would be quicker if the array was initialized to the correct size in the first place so that resizes are not necessary.
However, preallocating space for children is only necessary when performing a deep clone. Therefore, an additional parameter is being added to the Clone, CopyInnerTo, and CloneDocHelper methods to indicate whether preallocation of children should happen. Attributes are copied either way, so that part of the array is preallocated in both cases.
MozReview-Commit-ID: 3iVezeAKXnI
--HG--
extra : rebase_source : 9c3deec6d7aafd6411044d623d4863637b45fd58
The test doesn't verify that the exceptions are reported to the browser console,
but I manually checked that they are. Adding a test for that part could be
pretty annoying.
The test doesn't verify that the exceptions are reported to the browser console,
but I manually checked that they are. Adding a test for that part could be
pretty annoying.
It's not only inefficient, but also prone to buggyness. Since styles may not be
up-to-date when it happens.
Post a reconstruct instead, which ensures a style flush happens before running
frame construction.
MozReview-Commit-ID: DrakHsJv5fY
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
--HG--
extra : rebase_source : 11900af908654336cc2391ab9480542c5474e38f
This patch is generated by the following sed script:
find . ! -wholename '*/.hg*' -type f \( -iname '*.html' -o -iname '*.xhtml' -o -iname '*.xul' -o -iname '*.js' \) -exec sed -i -e 's/\(\(text\|application\)\/javascript\);version=1.[0-9]/\1/g' {} \;
MozReview-Commit-ID: AzhtdwJwVNg
--HG--
extra : rebase_source : e8f90249454c0779d926f87777f457352961748d
I noticed that our current behavior in ContentRangeInserted is incorrect. Unlike
ContentInserted (where this code lived originally), ContentRangeInserted takes a
start and end element. I'm not sure if we ever take that path for new content that
needs style, but it seemed sketchy. And generally, it seems nice to just always
style new content the same way (though we still need to style NAC by the subtree
root, since it hasn't been attached to the parent yet).
For situations where there is indeed only one unstyled child, the traversal
overhead should be neglible, since we special-case the single-element in
parallel.rs to avoid calling into rayon.
Being more explicit about what we want here also makes us more robust against
the other handful of callpaths that can take us into
nsCSSFrameConstructor::{ContentRangeInserted,ContentAppended}. Currently we
can call StyleNewSubtree on an already-styled element via RecreateFramesForContent,
which triggers an assertion in the servo traversal.
MozReview-Commit-ID: DqCGh90deHH
This patch implements some helper methods to log constants related to event handling.
ToString(KeyNameIndex) and ToString(CodeNameIndex) converts the enum itmes to human readable string. They use WidgetKeyboardEvent's helper class which returns Unicode text. Therefore, this need to convert from UTF16 to UTF8. That's the reason why these methods don't return |const char*|.
GetDOMKeyCodeName(uint32_t) returns DOM keycode name if it's defined. Otherwise, returns hexadecimal value. For generating switch-case statement, VirtualKeyCodeList.h shouldn't include ",". Therefore, this patch removes "," from VirtualKeyCodeList.h and append it at defining NS_DEFINE_VK. Additionally, the last item of enum and array should not end with ",". Therefore, this adds dummy last item at each of them. Finally, some of the keyCode values are shared between 2 keys. Therefore, it needs to support NS_DISALLOW_SAME_KEYCODE for switch-case generator. See the comment in the file for more detail.
GetModifiersName(Modifiers) returns all modifier names included in the given value.
MozReview-Commit-ID: 9i2ftFOTpDn
--HG--
extra : rebase_source : 458a4d28624dc10dd4454f2e7708d746d1fcb045
This change avoids lots of false positives for Coverity's CHECKED_RETURN
warning, caused by NS_WARN_IF's current use in both statement-style and
expression-style.
In the case where the code within the NS_WARN_IF has side-effects, I made the
following change.
> NS_WARN_IF(NS_FAILED(FunctionWithSideEffects()));
> -->
> Unused << NS_WARN_IF(NS_FAILED(FunctionWithSideEffects()));
In the case where the code within the NS_WARN_IF lacks side-effects, I made the
following change.
> NS_WARN_IF(!condWithoutSideEffects);
> -->
> NS_WARNING_ASSERTION(condWithoutSideEffects, "msg");
This has two improvements.
- The condition is not evaluated in non-debug builds.
- The sense of the condition is inverted to the familiar "this condition should
be true" sense used in assertions.
A common variation on the side-effect-free case is the following.
> nsresult rv = Fn();
> NS_WARN_IF_(NS_FAILED(rv));
> -->
> DebugOnly<nsresult rv> = Fn();
> NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Fn failed");
--HG--
extra : rebase_source : 58788245021096efa8372a9dc1d597a611d45611
This function is an infallible alternative to nsIURI::GetSpec(). It's useful
when it's appropriate to handle a GetSpec() failure with a failure string, e.g.
for log/warning/error messages. It allows code like this:
nsAutoCString spec;
uri->GetSpec(spec);
printf("uri: %s", spec.get());
to be changed to this:
printf("uri: %s", uri->GetSpecOrDefault().get());
This introduces a slight behavioural change. Previously, if GetSpec() failed,
an empty string would be used here. Now, "[nsIURI::GetSpec failed]" will be
produced instead. In most cases this failure string will make for a clearer
log/warning/error message than the empty string.
* * *
Bug 1297961 (part 1b) - More GetSpecOrDefault() additions. r=hurley.
I will fold this into part 1 before landing.
--HG--
extra : rebase_source : ddc19a5624354ac098be019ca13cc24b99b80ddc
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h