From 92f7e0e48e4a5cdb76d94bd997a9488c34d21945 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Sun, 12 Apr 2020 12:52:18 +0200 Subject: [PATCH 01/97] Issue #1512 - Improve handling of multiple selections. IsSelectionEditable should check whether the focus node and anchor node aren't null before trying to use them. This also changes the initialization of selections' aOutIndex to the last range in the selection as a fallback in case we don't add a range later (in AddItem) which could also end up with a null selection otherwise if the additional selection nodes are removed. --- editor/libeditor/HTMLEditor.cpp | 10 ++++++++-- layout/generic/nsSelection.cpp | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 368f7a3e91..532da7a150 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -3344,12 +3344,18 @@ HTMLEditor::GetIsSelectionEditable(bool* aIsSelectionEditable) RefPtr selection = GetSelection(); NS_ENSURE_TRUE(selection, NS_ERROR_NULL_POINTER); + nsINode* anchorNode = selection->GetAnchorNode(); + nsINode* focusNode = selection->GetFocusNode(); + if (!anchorNode || !focusNode) { + return NS_ERROR_FAILURE; + } + // Per the editing spec as of June 2012: we have to have a selection whose // start and end nodes are editable, and which share an ancestor editing // host. (Bug 766387.) *aIsSelectionEditable = selection->RangeCount() && - selection->GetAnchorNode()->IsEditable() && - selection->GetFocusNode()->IsEditable(); + anchorNode->IsEditable() && + focusNode->IsEditable(); if (*aIsSelectionEditable) { nsINode* commonAncestor = diff --git a/layout/generic/nsSelection.cpp b/layout/generic/nsSelection.cpp index f2959dc9dd..5ccb2d8bf8 100644 --- a/layout/generic/nsSelection.cpp +++ b/layout/generic/nsSelection.cpp @@ -3844,7 +3844,7 @@ Selection::AddItem(nsRange* aItem, int32_t* aOutIndex, bool aNoStartSelect) if (mUserInitiated) { AutoTArray, 4> rangesToAdd; - *aOutIndex = -1; + *aOutIndex = int32_t(mRanges.Length()) - 1; nsIDocument* doc = GetParentObject(); bool selectEventsEnabled = From 788e24053abc0406b4cb7d2e534ac71ef36aceb4 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 13 Apr 2020 12:55:47 +0200 Subject: [PATCH 02/97] Issue #1513 - Bail on orphan node This was refactored by Mozilla into doing effectively the same thing so it aligns with them re: behavior. Resolves #1513 --- editor/libeditor/HTMLEditRules.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/editor/libeditor/HTMLEditRules.cpp b/editor/libeditor/HTMLEditRules.cpp index c2d61f767b..545e22f701 100644 --- a/editor/libeditor/HTMLEditRules.cpp +++ b/editor/libeditor/HTMLEditRules.cpp @@ -4641,7 +4641,11 @@ HTMLEditRules::WillAlign(Selection& aSelection, } nsCOMPtr curParent = curNode->GetParentNode(); - int32_t offset = curParent ? curParent->IndexOf(curNode) : -1; + if (!curParent) { + continue; + } + + int32_t offset = curParent->IndexOf(curNode); // Skip insignificant formatting text nodes to prevent unnecessary // structure splitting! From de71a9e013b7db0c4284bf298e8b205a6cc2279c Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Mon, 13 Apr 2020 12:57:07 +0200 Subject: [PATCH 03/97] Issue #1514 - Perform validity check before trying to use the content. Resolves #1514 --- editor/libeditor/HTMLStyleEditor.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/editor/libeditor/HTMLStyleEditor.cpp b/editor/libeditor/HTMLStyleEditor.cpp index bc7141ad33..6a1ffe8b41 100644 --- a/editor/libeditor/HTMLStyleEditor.cpp +++ b/editor/libeditor/HTMLStyleEditor.cpp @@ -333,8 +333,9 @@ HTMLEditor::SetInlinePropertyOnTextNode(Text& aText, RefPtr text = &aText; if (uint32_t(aEndOffset) != aText.Length()) { // We need to split off back of text node - text = SplitNode(aText, aEndOffset, rv)->GetAsText(); + nsIContent* textNode = SplitNode(aText, aEndOffset, rv); NS_ENSURE_TRUE(!rv.Failed(), rv.StealNSResult()); + text = textNode->GetAsText(); } if (aStartOffset) { From 91c64d8c3f030809764a5b296f3f3777150b6b0f Mon Sep 17 00:00:00 2001 From: Moonchild Date: Mon, 13 Apr 2020 10:44:05 +0200 Subject: [PATCH 04/97] Create SECURITY.md Provide basic instructions how to report security vulnerabilities. --- SECURITY.md | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 SECURITY.md diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000000..af9c916cd0 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,36 @@ +# Security Policy + +## What is or is not a vulnerability? + +In general, vulnerabilities are those bugs that can actually be exploited to perform malicious tasks. +Most _crashes_ are not security vulnerabilities. Although important to fix, they don't inherently cause a problem for the +browser's security. + +What should be considered vulnerabilities or security hazards by default: +- Use-after-free crashes, since those can potentially be used for remote code execution; +- Spoofing issues in the UI; +- File security issues, like out-of-bounds access to arbitrary files or locations; +- Type confusion issues; +- Bypass of security measures like CSP or the various mechanisms around HTTPS. + +Generally not security vulnerabilities: +- Null dereferencing crashes; +- Malware extensions (but please do report those on the forum in the extensions board!); +- Denial-of-service (AKA "evil trap sites") +- Browser hangs +- Issues with non-standard manual configuration (either at build time or by manipulating about:config directly) + +## Reporting a Vulnerability + +If you find an issue in UXP or the applications it builds on that could impact the security or safety of users please **do not** +make an issue on GitHub about it. GitHub does not support restricted viewability for security sensitive bugs. + +If you want to report a security-sensitive issue then please go to the [forum](https://forum.palemoon.org) and report the issue +via a **private message** to **Moonchild** (the founder and prime responsible for security issues). +The forum's private message system is fully secure since your visits are encrypted and private messages are not available to anyone +except the recipient (not even moderators!). + +You will be informed via private message if the vulnerability report is accepted or declined, with reasoning. +Security updates occur regularly and are given priority over most other development tasks. In general, they can be solved +relatively quickly and will be included in the next point release (third digit if not rolled into a more major one). + From c3d2282ba86378fc3e24da8b52c7ab924b38693e Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Tue, 14 Apr 2020 19:16:07 -0400 Subject: [PATCH 05/97] Issue #1516 - Part 1: Add a global build define to determine if the client can draw in the titlebar and update consumers of the local define --- old-configure.in | 17 +++++++++++++++++ toolkit/modules/AppConstants.jsm | 2 +- toolkit/modules/LightweightThemeConsumer.jsm | 2 +- toolkit/modules/moz.build | 3 --- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/old-configure.in b/old-configure.in index f6d0fc46fa..ed0c320750 100644 --- a/old-configure.in +++ b/old-configure.in @@ -2176,6 +2176,7 @@ MOZ_LDAP_XPCOM= MOZ_MORK= MOZ_MAPI_SUPPORT= MOZ_INCOMPLETE_EXTERNAL_LINKAGE= +MOZ_CAN_DRAW_IN_TITLEBAR= case "$target_os" in mingw*) @@ -2824,6 +2825,22 @@ fi AC_SUBST(MOZ_MAPI_SUPPORT) +dnl ======================================================== +dnl = Client drawing in titlebar +dnl ======================================================== +if test -n "$MOZ_CAN_DRAW_IN_TITLEBAR"; then + case "$OS_TARGET" in + WINNT|Darwin) + AC_DEFINE(MOZ_CAN_DRAW_IN_TITLEBAR) + ;; + *) + AC_MSG_ERROR([Drawing in the titlebar is only supported on Windows and Macintosh targets]) + ;; + esac +fi + +AC_SUBST(MOZ_CAN_DRAW_IN_TITLEBAR) + dnl ======================================================== dnl = Enable WebRTC code dnl ======================================================== diff --git a/toolkit/modules/AppConstants.jsm b/toolkit/modules/AppConstants.jsm index 41b398efc1..40aaadbf07 100644 --- a/toolkit/modules/AppConstants.jsm +++ b/toolkit/modules/AppConstants.jsm @@ -209,7 +209,7 @@ MOZ_SAFE_BROWSING: #endif CAN_DRAW_IN_TITLEBAR: -#ifdef CAN_DRAW_IN_TITLEBAR +#ifdef MOZ_CAN_DRAW_IN_TITLEBAR true, #else false, diff --git a/toolkit/modules/LightweightThemeConsumer.jsm b/toolkit/modules/LightweightThemeConsumer.jsm index 325f711b78..4010a9ff22 100644 --- a/toolkit/modules/LightweightThemeConsumer.jsm +++ b/toolkit/modules/LightweightThemeConsumer.jsm @@ -147,7 +147,7 @@ LightweightThemeConsumer.prototype = { footer.removeAttribute("lwthemefooter"); } -#ifdef XP_MACOSX +#if defined(XP_MACOSX) && defined(MOZ_CAN_DRAW_IN_TITLEBAR) // On OS X, we extend the lightweight theme into the titlebar, which means setting // the chromemargin attribute. Some XUL applications already draw in the titlebar, // so we need to save the chromemargin value before we overwrite it with the value diff --git a/toolkit/modules/moz.build b/toolkit/modules/moz.build index 8a364df8c2..54adae4cf6 100644 --- a/toolkit/modules/moz.build +++ b/toolkit/modules/moz.build @@ -92,9 +92,6 @@ EXTRA_PP_JS_MODULES += [ 'WindowDraggingUtils.jsm', ] -if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('windows', 'cocoa'): - DEFINES['CAN_DRAW_IN_TITLEBAR'] = 1 - if CONFIG['MOZ_WIDGET_TOOLKIT'] in ('windows', 'gtk2', 'gtk3'): DEFINES['MENUBAR_CAN_AUTOHIDE'] = 1 From 78a9717bc3807cd1fff69ad5ee50ff26931c8eb1 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Thu, 16 Apr 2020 04:20:42 +0200 Subject: [PATCH 06/97] Issue #1517 - Enable high-resolution DOM timestamps on trunk At the very least we should enable these short term, with the potential removal of it pending. --- modules/libpref/init/all.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index f673ccc900..1e346b463a 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -1247,8 +1247,7 @@ pref("privacy.trackingprotection.pbmode.enabled", false); pref("dom.event.contextmenu.enabled", true); pref("dom.event.clipboardevents.enabled", true); -// Potentially risky option due to side channel attack surface using high res timers. -pref("dom.event.highrestimestamp.enabled", false); +pref("dom.event.highrestimestamp.enabled", true); pref("dom.webcomponents.enabled", false); pref("dom.webcomponents.customelements.enabled", false); From 20b2c264511bb3d4b4121ad1eac6e1b0d42a97f3 Mon Sep 17 00:00:00 2001 From: wolfbeast Date: Thu, 16 Apr 2020 23:15:33 +0200 Subject: [PATCH 07/97] Issue #251 - Move MOZ_AV1 back to ac configure for everyone to use. --- build/moz.configure/old.configure | 1 + old-configure.in | 15 +++++++++++++++ toolkit/moz.configure | 14 -------------- 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/build/moz.configure/old.configure b/build/moz.configure/old.configure index a2ce58cfc2..9d1a33c66e 100644 --- a/build/moz.configure/old.configure +++ b/build/moz.configure/old.configure @@ -157,6 +157,7 @@ def old_configure_options(*options): '--enable-accessibility', '--enable-address-sanitizer', '--enable-alsa', + '--enable-av1', '--enable-b2g-bt', '--enable-b2g-camera', '--enable-b2g-ril', diff --git a/old-configure.in b/old-configure.in index ed0c320750..660064bbfc 100644 --- a/old-configure.in +++ b/old-configure.in @@ -2122,6 +2122,7 @@ if test "$MOZ_IOS"; then MOZ_AUTH_EXTENSION= fi MOZ_RAW= +MOZ_AV1= MOZ_VORBIS= MOZ_TREMOR= MOZ_SAMPLE_TYPE_FLOAT32= @@ -2923,6 +2924,20 @@ fi AC_SUBST(MOZ_RAW) +dnl ======================================================== +dnl = Enable Libaom AV1 Codec +dnl ======================================================== +MOZ_ARG_ENABLE_BOOL(av1, +[ --enable-av1 Enable libaom for av1 video support], + MOZ_AV1=1, + MOZ_AV1=) + +if test -n "$MOZ_AV1"; then + AC_DEFINE(MOZ_AV1) +fi + +AC_SUBST(MOZ_AV1) + dnl ======================================================== dnl = Apple platform decoder support dnl ======================================================== diff --git a/toolkit/moz.configure b/toolkit/moz.configure index 128402257f..fe14cb5672 100644 --- a/toolkit/moz.configure +++ b/toolkit/moz.configure @@ -345,20 +345,6 @@ set_config('MOZ_FMP4', fmp4) set_define('MOZ_FMP4', fmp4) add_old_configure_assignment('MOZ_FMP4', fmp4) -# Libaom AV1 Video Codec Support -# ============================================================== -option('--enable-av1', - help='Enable libaom for av1 video support') - -@depends('--enable-av1') -def av1(value): - enabled = bool(value) - if enabled: - return True - -set_config('MOZ_AV1', av1) -set_define('MOZ_AV1', av1) - # Miscellaneous # ============================================================== option(name='--enable-chrome-format', From cb2b45e472b8b369b6eb4fc6f4e3f402600cf70b Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Tue, 14 Apr 2020 21:24:51 -0400 Subject: [PATCH 08/97] Bug 1305458 - Changing -moz-appearence on hover breaks change event * Rename nsIDOMEventTarget::PreHandleEvent to nsIDOMEventTarget::GetEventTargetParent * Add nsIDOMEventTarget::PreHandleEvent * Add EventTargetChainItem::GetFirstEventTarget * Call EventTargetChainItem::PreHandleEvent even it sets mCanHandle=false * Move form control frame focus/blur from nsGenericHTMLFormElement::GetEventTargetParent to PreHandleEvent * Move fire change event from HTMLTextAreaElement::GetEventTargetParent to PreHandleEvent * Refine nsXULElement::GetEventTargetParent * Move dispatch XUL command from nsXULElement::GetEventTargetParent to PreHandleEvent * Move fire events and set value from HTMLInputElement::GetEventTargetParent to PreHandleEvent * Add test case * Let HTMLInputElement delegate event handling to it's parent class * Refine EventTargetChain flags to reduce overheads * Refine event target chain creation * Refine assertion in EventTargetChainItem::Create Tag #1375 --- dom/archivereader/ArchiveRequest.cpp | 2 +- dom/archivereader/ArchiveRequest.h | 3 +- dom/base/Attr.cpp | 2 +- dom/base/Attr.h | 2 +- dom/base/Element.cpp | 7 +- dom/base/Element.h | 2 +- dom/base/FragmentOrElement.cpp | 2 +- dom/base/nsDocument.cpp | 2 +- dom/base/nsDocument.h | 2 +- dom/base/nsGenConImageContent.cpp | 4 +- dom/base/nsGlobalWindow.cpp | 7 +- dom/base/nsIContent.h | 2 +- dom/base/nsINode.cpp | 2 +- dom/base/nsInProcessTabChildGlobal.cpp | 4 +- dom/base/nsInProcessTabChildGlobal.h | 4 +- dom/base/nsWindowRoot.cpp | 2 +- dom/events/DOMEventTargetHelper.cpp | 2 +- dom/events/DOMEventTargetHelper.h | 4 +- dom/events/EventDispatcher.cpp | 252 ++++++++++++++------ dom/events/EventDispatcher.h | 24 +- dom/events/test/mochitest.ini | 1 + dom/events/test/test_bug1305458.html | 50 ++++ dom/html/HTMLAnchorElement.cpp | 4 +- dom/html/HTMLAnchorElement.h | 3 +- dom/html/HTMLAreaElement.cpp | 4 +- dom/html/HTMLAreaElement.h | 3 +- dom/html/HTMLButtonElement.cpp | 4 +- dom/html/HTMLButtonElement.h | 3 +- dom/html/HTMLCanvasElement.cpp | 4 +- dom/html/HTMLCanvasElement.h | 3 +- dom/html/HTMLFieldSetElement.cpp | 4 +- dom/html/HTMLFieldSetElement.h | 3 +- dom/html/HTMLFormElement.cpp | 4 +- dom/html/HTMLFormElement.h | 3 +- dom/html/HTMLImageElement.cpp | 4 +- dom/html/HTMLImageElement.h | 3 +- dom/html/HTMLInputElement.cpp | 90 ++++--- dom/html/HTMLInputElement.h | 4 +- dom/html/HTMLLinkElement.cpp | 4 +- dom/html/HTMLLinkElement.h | 3 +- dom/html/HTMLMenuItemElement.cpp | 4 +- dom/html/HTMLMenuItemElement.h | 3 +- dom/html/HTMLOptGroupElement.cpp | 4 +- dom/html/HTMLOptGroupElement.h | 3 +- dom/html/HTMLSelectElement.cpp | 4 +- dom/html/HTMLSelectElement.h | 3 +- dom/html/HTMLTextAreaElement.cpp | 17 +- dom/html/HTMLTextAreaElement.h | 4 +- dom/html/nsGenericHTMLElement.cpp | 21 +- dom/html/nsGenericHTMLElement.h | 9 +- dom/indexedDB/IDBFileHandle.cpp | 2 +- dom/indexedDB/IDBFileHandle.h | 2 +- dom/indexedDB/IDBFileRequest.cpp | 2 +- dom/indexedDB/IDBFileRequest.h | 2 +- dom/indexedDB/IDBRequest.cpp | 2 +- dom/indexedDB/IDBRequest.h | 2 +- dom/indexedDB/IDBTransaction.cpp | 2 +- dom/indexedDB/IDBTransaction.h | 2 +- dom/interfaces/events/nsIDOMEventTarget.idl | 15 +- dom/ipc/TabChild.h | 2 +- dom/mathml/nsMathMLElement.cpp | 6 +- dom/mathml/nsMathMLElement.h | 2 +- dom/svg/SVGAElement.cpp | 6 +- dom/svg/SVGAElement.h | 3 +- dom/svg/SVGSVGElement.cpp | 4 +- dom/svg/SVGSVGElement.h | 3 +- dom/workers/SharedWorker.cpp | 4 +- dom/workers/SharedWorker.h | 2 +- dom/xul/nsXULElement.cpp | 124 ++++++---- dom/xul/nsXULElement.h | 10 +- 70 files changed, 545 insertions(+), 256 deletions(-) create mode 100644 dom/events/test/test_bug1305458.html diff --git a/dom/archivereader/ArchiveRequest.cpp b/dom/archivereader/ArchiveRequest.cpp index ec16868042..c4b19f56bc 100644 --- a/dom/archivereader/ArchiveRequest.cpp +++ b/dom/archivereader/ArchiveRequest.cpp @@ -69,7 +69,7 @@ ArchiveRequest::~ArchiveRequest() } nsresult -ArchiveRequest::PreHandleEvent(EventChainPreVisitor& aVisitor) +ArchiveRequest::GetEventTargetParent(EventChainPreVisitor& aVisitor) { aVisitor.mCanHandle = true; aVisitor.mParentTarget = nullptr; diff --git a/dom/archivereader/ArchiveRequest.h b/dom/archivereader/ArchiveRequest.h index 3988f1aa15..3081a3e420 100644 --- a/dom/archivereader/ArchiveRequest.h +++ b/dom/archivereader/ArchiveRequest.h @@ -38,7 +38,8 @@ public: ArchiveReader* aReader); // nsIDOMEventTarget - virtual nsresult PreHandleEvent(EventChainPreVisitor& aVisitor) override; + virtual nsresult GetEventTargetParent( + EventChainPreVisitor& aVisitor) override; public: // This is called by the DOMArchiveRequestEvent diff --git a/dom/base/Attr.cpp b/dom/base/Attr.cpp index 71b559392b..df05ef5cb6 100644 --- a/dom/base/Attr.cpp +++ b/dom/base/Attr.cpp @@ -344,7 +344,7 @@ Attr::RemoveChildAt(uint32_t aIndex, bool aNotify) } nsresult -Attr::PreHandleEvent(EventChainPreVisitor& aVisitor) +Attr::GetEventTargetParent(EventChainPreVisitor& aVisitor) { aVisitor.mCanHandle = true; return NS_OK; diff --git a/dom/base/Attr.h b/dom/base/Attr.h index 3f80030d22..39ac8b1955 100644 --- a/dom/base/Attr.h +++ b/dom/base/Attr.h @@ -54,7 +54,7 @@ public: // nsIDOMAttr interface NS_DECL_NSIDOMATTR - virtual nsresult PreHandleEvent(EventChainPreVisitor& aVisitor) override; + virtual nsresult GetEventTargetParent(EventChainPreVisitor& aVisitor) override; // nsIAttribute interface void SetMap(nsDOMAttributeMap *aMap) override; diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index c8467e0365..5a92853ae4 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -3129,7 +3129,7 @@ Element::CheckHandleEventForLinksPrecondition(EventChainVisitor& aVisitor, } nsresult -Element::PreHandleEventForLinks(EventChainPreVisitor& aVisitor) +Element::GetEventTargetParentForLinks(EventChainPreVisitor& aVisitor) { // Optimisation: return early if this event doesn't interest us. // IMPORTANT: this switch and the switch below it must be kept in sync! @@ -3151,8 +3151,9 @@ Element::PreHandleEventForLinks(EventChainPreVisitor& aVisitor) nsresult rv = NS_OK; - // We do the status bar updates in PreHandleEvent so that the status bar gets - // updated even if the event is consumed before we have a chance to set it. + // We do the status bar updates in GetEventTargetParent so that the status bar + // gets updated even if the event is consumed before we have a chance to set + // it. switch (aVisitor.mEvent->mMessage) { // Set the status bar similarly for mouseover and focus case eMouseOver: diff --git a/dom/base/Element.h b/dom/base/Element.h index 7820047034..0104d795c2 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -1400,7 +1400,7 @@ protected: /** * Handle status bar updates before they can be cancelled. */ - nsresult PreHandleEventForLinks(EventChainPreVisitor& aVisitor); + nsresult GetEventTargetParentForLinks(EventChainPreVisitor& aVisitor); /** * Handle default actions for link event if the event isn't consumed yet. diff --git a/dom/base/FragmentOrElement.cpp b/dom/base/FragmentOrElement.cpp index 526c3c9d44..ca00a49a5e 100644 --- a/dom/base/FragmentOrElement.cpp +++ b/dom/base/FragmentOrElement.cpp @@ -718,7 +718,7 @@ FindChromeAccessOnlySubtreeOwner(nsIContent* aContent) } nsresult -nsIContent::PreHandleEvent(EventChainPreVisitor& aVisitor) +nsIContent::GetEventTargetParent(EventChainPreVisitor& aVisitor) { //FIXME! Document how this event retargeting works, Bug 329124. aVisitor.mCanHandle = true; diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index b05bf827bf..45c80ca7f6 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -7712,7 +7712,7 @@ nsDocument::GetExistingListenerManager() const } nsresult -nsDocument::PreHandleEvent(EventChainPreVisitor& aVisitor) +nsDocument::GetEventTargetParent(EventChainPreVisitor& aVisitor) { aVisitor.mCanHandle = true; // FIXME! This is a hack to make middle mouse paste working also in Editor. diff --git a/dom/base/nsDocument.h b/dom/base/nsDocument.h index 90e511dcbb..79b7f18f4f 100644 --- a/dom/base/nsDocument.h +++ b/dom/base/nsDocument.h @@ -810,7 +810,7 @@ public: NS_DECL_NSIDOMDOCUMENTXBL // nsIDOMEventTarget - virtual nsresult PreHandleEvent( + virtual nsresult GetEventTargetParent( mozilla::EventChainPreVisitor& aVisitor) override; virtual mozilla::EventListenerManager* GetOrCreateListenerManager() override; diff --git a/dom/base/nsGenConImageContent.cpp b/dom/base/nsGenConImageContent.cpp index e1b1f5a179..af3b186bdc 100644 --- a/dom/base/nsGenConImageContent.cpp +++ b/dom/base/nsGenConImageContent.cpp @@ -46,7 +46,7 @@ public: virtual void UnbindFromTree(bool aDeep, bool aNullParent) override; virtual EventStates IntrinsicState() const override; - virtual nsresult PreHandleEvent(EventChainPreVisitor& aVisitor) override + virtual nsresult GetEventTargetParent(EventChainPreVisitor& aVisitor) override { MOZ_ASSERT(IsInNativeAnonymousSubtree()); if (aVisitor.mEvent->mMessage == eLoad || @@ -54,7 +54,7 @@ public: // Don't propagate the events to the parent. return NS_OK; } - return nsXMLElement::PreHandleEvent(aVisitor); + return nsXMLElement::GetEventTargetParent(aVisitor); } private: diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index c965d5b974..802b44f381 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -3577,9 +3577,10 @@ nsGlobalWindow::WillHandleEvent(EventChainPostVisitor& aVisitor) } nsresult -nsGlobalWindow::PreHandleEvent(EventChainPreVisitor& aVisitor) +nsGlobalWindow::GetEventTargetParent(EventChainPreVisitor& aVisitor) { - NS_PRECONDITION(IsInnerWindow(), "PreHandleEvent is used on outer window!?"); + NS_PRECONDITION(IsInnerWindow(), + "GetEventTargetParent is used on outer window!?"); EventMessage msg = aVisitor.mEvent->mMessage; aVisitor.mCanHandle = true; @@ -3812,7 +3813,7 @@ nsGlobalWindow::PostHandleEvent(EventChainPostVisitor& aVisitor) } else if (aVisitor.mEvent->mMessage == eLoad && aVisitor.mEvent->IsTrusted()) { // This is page load event since load events don't propagate to |window|. - // @see nsDocument::PreHandleEvent. + // @see nsDocument::GetEventTargetParent. mIsDocumentLoaded = true; nsCOMPtr element = GetOuterWindow()->GetFrameElementInternal(); diff --git a/dom/base/nsIContent.h b/dom/base/nsIContent.h index dcdc632b49..e179d6ebce 100644 --- a/dom/base/nsIContent.h +++ b/dom/base/nsIContent.h @@ -947,7 +947,7 @@ public: // Overloaded from nsINode virtual already_AddRefed GetBaseURI(bool aTryUseXHRDocBaseURI = false) const override; - virtual nsresult PreHandleEvent( + virtual nsresult GetEventTargetParent( mozilla::EventChainPreVisitor& aVisitor) override; virtual bool IsPurple() = 0; diff --git a/dom/base/nsINode.cpp b/dom/base/nsINode.cpp index 212110b729..0b56f519d0 100644 --- a/dom/base/nsINode.cpp +++ b/dom/base/nsINode.cpp @@ -1243,7 +1243,7 @@ nsINode::RemoveEventListener(const nsAString& aType, NS_IMPL_REMOVE_SYSTEM_EVENT_LISTENER(nsINode) nsresult -nsINode::PreHandleEvent(EventChainPreVisitor& aVisitor) +nsINode::GetEventTargetParent(EventChainPreVisitor& aVisitor) { // This is only here so that we can use the NS_DECL_NSIDOMTARGET macro NS_ABORT(); diff --git a/dom/base/nsInProcessTabChildGlobal.cpp b/dom/base/nsInProcessTabChildGlobal.cpp index 10ccf4aec3..547bb8d36b 100644 --- a/dom/base/nsInProcessTabChildGlobal.cpp +++ b/dom/base/nsInProcessTabChildGlobal.cpp @@ -97,7 +97,7 @@ nsInProcessTabChildGlobal::nsInProcessTabChildGlobal(nsIDocShell* aShell, mozilla::HoldJSObjects(this); // If owner corresponds to an + + \ No newline at end of file diff --git a/dom/html/reftests/body-frame-margin-remove-other-pres-hint.html b/dom/html/reftests/body-frame-margin-remove-other-pres-hint.html new file mode 100644 index 0000000000..6f49536376 --- /dev/null +++ b/dom/html/reftests/body-frame-margin-remove-other-pres-hint.html @@ -0,0 +1,16 @@ + + + + + + + + + + \ No newline at end of file diff --git a/dom/html/reftests/body-topmargin-dynamic.html b/dom/html/reftests/body-topmargin-dynamic.html new file mode 100644 index 0000000000..e6c8c505e7 --- /dev/null +++ b/dom/html/reftests/body-topmargin-dynamic.html @@ -0,0 +1,12 @@ + + + +this text should have a margin of 100px on the top and left +

this text should have a margin of 100px on the right

+ + + diff --git a/dom/html/reftests/body-topmargin-ref.html b/dom/html/reftests/body-topmargin-ref.html new file mode 100644 index 0000000000..6530a0ae4b --- /dev/null +++ b/dom/html/reftests/body-topmargin-ref.html @@ -0,0 +1,7 @@ + + + +this text should have a margin of 100px on the top and left +

this text should have a margin of 100px on the right

+ + diff --git a/dom/html/reftests/reftest-stylo.list b/dom/html/reftests/reftest-stylo.list index dd6339edf8..570c073587 100644 --- a/dom/html/reftests/reftest-stylo.list +++ b/dom/html/reftests/reftest-stylo.list @@ -64,3 +64,10 @@ pref(permissions.default.image,2) HTTP == bug1196784-with-srcset.html bug1196784 # Test video with rotation information can be rotated. == bug1228601-video-rotation-90.html bug1228601-video-rotation-90.html + +# Test that dynamically setting body margin attributes updates style appropriately +== body-topmargin-dynamic.html body-topmargin-dynamic.html + +# Test that dynamically removing a nonmargin mapped attribute does not +# destroy margins inherited from the frame. +== body-frame-margin-remove-other-pres-hint.html body-frame-margin-remove-other-pres-hint.html diff --git a/dom/html/reftests/reftest.list b/dom/html/reftests/reftest.list index 27a13e7c92..82e773abb5 100644 --- a/dom/html/reftests/reftest.list +++ b/dom/html/reftests/reftest.list @@ -62,3 +62,10 @@ pref(permissions.default.image,2) HTTP == bug1196784-with-srcset.html bug1196784 # Test video with rotation information can be rotated. == bug1228601-video-rotation-90.html bug1228601-video-rotated-ref.html + +# Test that dynamically setting body margin attributes updates style appropriately +== body-topmargin-dynamic.html body-topmargin-ref.html + +# Test that dynamically removing a nonmargin mapped attribute does not +# destroy margins inherited from the frame. +== body-frame-margin-remove-other-pres-hint.html body-frame-margin-remove-other-pres-hint-ref.html From d3ffda520f97a1f7cedaa6445091b7c2f84de5c6 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:10:25 -0400 Subject: [PATCH 27/97] Bug 1355351 - Make pseudo-elements return the correct style via getComputedStyle * Add a node property to access the ::before and ::after pseudo-elements * Look for the frame for ::before and ::after pseudos * Clean up pseudo-element props * Simplify nsLayoutUtils callers, and make child iterators notice display: contents pseudos Tag #1375 --- .../browser_computed_pseudo-element_01.js | 4 +- dom/animation/EffectCompositor.cpp | 22 ++--- dom/animation/KeyframeEffectReadOnly.cpp | 12 +-- dom/base/ChildIterator.cpp | 73 +++++--------- dom/base/nsDOMWindowUtils.cpp | 6 +- dom/base/nsGkAtomList.h | 2 + dom/xml/nsXMLElement.cpp | 23 +++++ dom/xml/nsXMLElement.h | 3 + layout/base/RestyleManager.cpp | 4 +- layout/base/ServoRestyleManager.cpp | 13 +-- layout/base/nsCSSFrameConstructor.cpp | 61 ++++++------ layout/base/nsLayoutUtils.cpp | 98 +++++-------------- layout/base/nsLayoutUtils.h | 44 ++------- layout/generic/nsFrame.cpp | 19 ++-- layout/generic/nsIFrame.h | 4 - layout/style/nsComputedDOMStyle.cpp | 47 ++++++++- 16 files changed, 191 insertions(+), 244 deletions(-) diff --git a/devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js b/devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js index 9ca5451a56..7c18bfe7bf 100644 --- a/devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js +++ b/devtools/client/inspector/computed/test/browser_computed_pseudo-element_01.js @@ -33,7 +33,7 @@ function* testTopLeft(inspector, view) { let afterElement = children.nodes[children.nodes.length - 1]; yield selectNode(afterElement, inspector); top = getComputedViewPropertyValue(view, "top"); - is(top, "50%", "The computed view shows the correct top"); + is(top, "96px", "The computed view shows the correct top"); left = getComputedViewPropertyValue(view, "left"); - is(left, "50%", "The computed view shows the correct left"); + is(left, "96px", "The computed view shows the correct left"); } diff --git a/dom/animation/EffectCompositor.cpp b/dom/animation/EffectCompositor.cpp index c88cabe90b..49da3c0332 100644 --- a/dom/animation/EffectCompositor.cpp +++ b/dom/animation/EffectCompositor.cpp @@ -356,24 +356,16 @@ EffectCompositor::GetElementToRestyle(dom::Element* aElement, return aElement; } - nsIFrame* primaryFrame = aElement->GetPrimaryFrame(); - if (!primaryFrame) { - return nullptr; - } - nsIFrame* pseudoFrame; if (aPseudoType == CSSPseudoElementType::before) { - pseudoFrame = nsLayoutUtils::GetBeforeFrame(primaryFrame); - } else if (aPseudoType == CSSPseudoElementType::after) { - pseudoFrame = nsLayoutUtils::GetAfterFrame(primaryFrame); - } else { - NS_NOTREACHED("Should not try to get the element to restyle for a pseudo " - "other that :before or :after"); - return nullptr; + return nsLayoutUtils::GetBeforePseudo(aElement); } - if (!pseudoFrame) { - return nullptr; + if (aPseudoType == CSSPseudoElementType::after) { + return nsLayoutUtils::GetAfterPseudo(aElement); } - return pseudoFrame->GetContent()->AsElement(); + + NS_NOTREACHED("Should not try to get the element to restyle for a pseudo " + "other that :before or :after"); + return nullptr; } bool diff --git a/dom/animation/KeyframeEffectReadOnly.cpp b/dom/animation/KeyframeEffectReadOnly.cpp index 639e0b2b0d..164ee02925 100644 --- a/dom/animation/KeyframeEffectReadOnly.cpp +++ b/dom/animation/KeyframeEffectReadOnly.cpp @@ -1105,19 +1105,17 @@ KeyframeEffectReadOnly::GetAnimationFrame() const return nullptr; } - nsIFrame* frame = mTarget->mElement->GetPrimaryFrame(); - if (!frame) { - return nullptr; - } - + nsIFrame* frame; if (mTarget->mPseudoType == CSSPseudoElementType::before) { - frame = nsLayoutUtils::GetBeforeFrame(frame); + frame = nsLayoutUtils::GetBeforeFrame(mTarget->mElement); } else if (mTarget->mPseudoType == CSSPseudoElementType::after) { - frame = nsLayoutUtils::GetAfterFrame(frame); + frame = nsLayoutUtils::GetAfterFrame(mTarget->mElement); } else { + frame = mTarget->mElement->GetPrimaryFrame(); MOZ_ASSERT(mTarget->mPseudoType == CSSPseudoElementType::NotPseudo, "unknown mTarget->mPseudoType"); } + if (!frame) { return nullptr; } diff --git a/dom/base/ChildIterator.cpp b/dom/base/ChildIterator.cpp index fb07e9a210..23b919516b 100644 --- a/dom/base/ChildIterator.cpp +++ b/dom/base/ChildIterator.cpp @@ -317,11 +317,9 @@ AllChildrenIterator::Get() const { switch (mPhase) { case eAtBeforeKid: { - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - MOZ_ASSERT(frame, "No frame at eAtBeforeKid phase"); - nsIFrame* beforeFrame = nsLayoutUtils::GetBeforeFrame(frame); - MOZ_ASSERT(beforeFrame, "No content before frame at eAtBeforeKid phase"); - return beforeFrame->GetContent(); + Element* before = nsLayoutUtils::GetBeforePseudo(mOriginalContent); + MOZ_ASSERT(before, "No content before frame at eAtBeforeKid phase"); + return before; } case eAtExplicitKids: @@ -331,11 +329,9 @@ AllChildrenIterator::Get() const return mAnonKids[mAnonKidsIdx]; case eAtAfterKid: { - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - MOZ_ASSERT(frame, "No frame at eAtAfterKid phase"); - nsIFrame* afterFrame = nsLayoutUtils::GetAfterFrame(frame); - MOZ_ASSERT(afterFrame, "No content before frame at eAtBeforeKid phase"); - return afterFrame->GetContent(); + Element* after = nsLayoutUtils::GetAfterPseudo(mOriginalContent); + MOZ_ASSERT(after, "No content after frame at eAtAfterKid phase"); + return after; } default: @@ -349,15 +345,10 @@ AllChildrenIterator::Seek(nsIContent* aChildToFind) { if (mPhase == eAtBegin || mPhase == eAtBeforeKid) { mPhase = eAtExplicitKids; - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - if (frame) { - nsIFrame* beforeFrame = nsLayoutUtils::GetBeforeFrame(frame); - if (beforeFrame) { - if (beforeFrame->GetContent() == aChildToFind) { - mPhase = eAtBeforeKid; - return true; - } - } + Element* beforePseudo = nsLayoutUtils::GetBeforePseudo(mOriginalContent); + if (beforePseudo && beforePseudo == aChildToFind) { + mPhase = eAtBeforeKid; + return true; } } @@ -404,13 +395,10 @@ AllChildrenIterator::GetNextChild() { if (mPhase == eAtBegin) { mPhase = eAtExplicitKids; - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - if (frame) { - nsIFrame* beforeFrame = nsLayoutUtils::GetBeforeFrame(frame); - if (beforeFrame) { - mPhase = eAtBeforeKid; - return beforeFrame->GetContent(); - } + Element* beforeContent = nsLayoutUtils::GetBeforePseudo(mOriginalContent); + if (beforeContent) { + mPhase = eAtBeforeKid; + return beforeContent; } } @@ -446,13 +434,10 @@ AllChildrenIterator::GetNextChild() return mAnonKids[mAnonKidsIdx]; } - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - if (frame) { - nsIFrame* afterFrame = nsLayoutUtils::GetAfterFrame(frame); - if (afterFrame) { - mPhase = eAtAfterKid; - return afterFrame->GetContent(); - } + Element* afterContent = nsLayoutUtils::GetAfterPseudo(mOriginalContent); + if (afterContent) { + mPhase = eAtAfterKid; + return afterContent; } } @@ -466,13 +451,10 @@ AllChildrenIterator::GetPreviousChild() if (mPhase == eAtEnd) { MOZ_ASSERT(mAnonKidsIdx == mAnonKids.Length()); mPhase = eAtAnonKids; - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - if (frame) { - nsIFrame* afterFrame = nsLayoutUtils::GetAfterFrame(frame); - if (afterFrame) { - mPhase = eAtAfterKid; - return afterFrame->GetContent(); - } + Element* afterContent = nsLayoutUtils::GetAfterPseudo(mOriginalContent); + if (afterContent) { + mPhase = eAtAfterKid; + return afterContent; } } @@ -501,13 +483,10 @@ AllChildrenIterator::GetPreviousChild() return kid; } - nsIFrame* frame = mOriginalContent->GetPrimaryFrame(); - if (frame) { - nsIFrame* beforeFrame = nsLayoutUtils::GetBeforeFrame(frame); - if (beforeFrame) { - mPhase = eAtBeforeKid; - return beforeFrame->GetContent(); - } + Element* beforeContent = nsLayoutUtils::GetBeforePseudo(mOriginalContent); + if (beforeContent) { + mPhase = eAtBeforeKid; + return beforeContent; } } diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index 2ab5937ace..3f5b54c942 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -3657,11 +3657,11 @@ nsDOMWindowUtils::GetOMTAStyle(nsIDOMElement* aElement, RefPtr cssValue = nullptr; nsIFrame* frame = element->GetPrimaryFrame(); - if (frame && !aPseudoElement.IsEmpty()) { + if (!aPseudoElement.IsEmpty()) { if (aPseudoElement.EqualsLiteral("::before")) { - frame = nsLayoutUtils::GetBeforeFrame(frame); + frame = nsLayoutUtils::GetBeforeFrame(element); } else if (aPseudoElement.EqualsLiteral("::after")) { - frame = nsLayoutUtils::GetAfterFrame(frame); + frame = nsLayoutUtils::GetAfterFrame(element); } else { return NS_ERROR_INVALID_ARG; } diff --git a/dom/base/nsGkAtomList.h b/dom/base/nsGkAtomList.h index c7c1197b1a..2f0e30387c 100644 --- a/dom/base/nsGkAtomList.h +++ b/dom/base/nsGkAtomList.h @@ -2144,12 +2144,14 @@ GK_ATOM(ongamepaddisconnected, "ongamepaddisconnected") #endif // Content property names +GK_ATOM(afterPseudoProperty, "afterPseudoProperty") // nsXMLElement* GK_ATOM(animationsProperty, "AnimationsProperty") // FrameAnimations* GK_ATOM(animationsOfBeforeProperty, "AnimationsOfBeforeProperty") // FrameAnimations* GK_ATOM(animationsOfAfterProperty, "AnimationsOfAfterProperty") // FrameAnimations* GK_ATOM(animationEffectsProperty, "AnimationEffectsProperty") // EffectSet* GK_ATOM(animationEffectsForBeforeProperty, "AnimationsEffectsForBeforeProperty") // EffectSet* GK_ATOM(animationEffectsForAfterProperty, "AnimationsEffectsForAfterProperty") // EffectSet* +GK_ATOM(beforePseudoProperty, "beforePseudoProperty") // nsXMLElement* GK_ATOM(cssPseudoElementBeforeProperty, "CSSPseudoElementBeforeProperty") // CSSPseudoElement* GK_ATOM(cssPseudoElementAfterProperty, "CSSPseudoElementAfterProperty") // CSSPseudoElement* GK_ATOM(transitionsProperty, "TransitionsProperty") // FrameTransitions* diff --git a/dom/xml/nsXMLElement.cpp b/dom/xml/nsXMLElement.cpp index 45163a1c42..ad0314f4ca 100644 --- a/dom/xml/nsXMLElement.cpp +++ b/dom/xml/nsXMLElement.cpp @@ -9,6 +9,7 @@ #include "mozilla/dom/ElementInlines.h" #include "nsContentUtils.h" // nsAutoScriptBlocker +using namespace mozilla; using namespace mozilla::dom; nsresult @@ -29,4 +30,26 @@ nsXMLElement::WrapNode(JSContext *aCx, JS::Handle aGivenProto) return ElementBinding::Wrap(aCx, this, aGivenProto); } +void +nsXMLElement::UnbindFromTree(bool aDeep, bool aNullParent) +{ + CSSPseudoElementType pseudoType = GetPseudoElementType(); + bool isBefore = pseudoType == CSSPseudoElementType::before; + nsIAtom* property = isBefore + ? nsGkAtoms::beforePseudoProperty : nsGkAtoms::afterPseudoProperty; + + switch (pseudoType) { + case CSSPseudoElementType::before: + case CSSPseudoElementType::after: { + MOZ_ASSERT(GetParent()); + MOZ_ASSERT(GetParent()->IsElement()); + GetParent()->DeleteProperty(property); + break; + } + default: + break; + } + Element::UnbindFromTree(aDeep, aNullParent); +} + NS_IMPL_ELEMENT_CLONE(nsXMLElement) diff --git a/dom/xml/nsXMLElement.h b/dom/xml/nsXMLElement.h index d16b82f399..85f79970cd 100644 --- a/dom/xml/nsXMLElement.h +++ b/dom/xml/nsXMLElement.h @@ -35,6 +35,9 @@ public: virtual nsIDOMNode* AsDOMNode() override { return this; } + virtual void UnbindFromTree(bool aDeep = true, + bool aNullParent = true) override; + protected: virtual ~nsXMLElement() {} diff --git a/layout/base/RestyleManager.cpp b/layout/base/RestyleManager.cpp index 4d08dee067..8db3d93020 100644 --- a/layout/base/RestyleManager.cpp +++ b/layout/base/RestyleManager.cpp @@ -3626,14 +3626,14 @@ ElementRestyler::MustReframeForPseudo(CSSPseudoElementType aPseudoType, // Check for a ::before pseudo style and the absence of a ::before content, // but only if aFrame is null or is the first continuation/ib-split. if ((aFrame && !nsLayoutUtils::IsFirstContinuationOrIBSplitSibling(aFrame)) || - nsLayoutUtils::GetBeforeFrameForContent(aGenConParentFrame, aContent)) { + nsLayoutUtils::GetBeforeFrame(aContent)) { return false; } } else { // Similarly for ::after, but check for being the last continuation/ // ib-split. if ((aFrame && nsLayoutUtils::GetNextContinuationOrIBSplitSibling(aFrame)) || - nsLayoutUtils::GetAfterFrameForContent(aGenConParentFrame, aContent)) { + nsLayoutUtils::GetAfterFrame(aContent)) { return false; } } diff --git a/layout/base/ServoRestyleManager.cpp b/layout/base/ServoRestyleManager.cpp index 9624c678be..0659ab8571 100644 --- a/layout/base/ServoRestyleManager.cpp +++ b/layout/base/ServoRestyleManager.cpp @@ -277,24 +277,17 @@ ServoRestyleManager::FrameForPseudoElement(const nsIContent* aContent, nsIAtom* aPseudoTagOrNull) { MOZ_ASSERT_IF(aPseudoTagOrNull, aContent->IsElement()); - nsIFrame* primaryFrame = aContent->GetPrimaryFrame(); if (!aPseudoTagOrNull) { - return primaryFrame; + return aContent->GetPrimaryFrame(); } - if (!primaryFrame) { - return nullptr; - } - - // NOTE: we probably need to special-case display: contents here. Gecko's - // RestyleManager passes the primary frame of the parent instead. if (aPseudoTagOrNull == nsCSSPseudoElements::before) { - return nsLayoutUtils::GetBeforeFrameForContent(primaryFrame, aContent); + return nsLayoutUtils::GetBeforeFrame(aContent); } if (aPseudoTagOrNull == nsCSSPseudoElements::after) { - return nsLayoutUtils::GetAfterFrameForContent(primaryFrame, aContent); + return nsLayoutUtils::GetAfterFrame(aContent); } MOZ_CRASH("Unkown pseudo-element given to " diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index b9778d36de..866fa148bf 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -1832,11 +1832,7 @@ nsCSSFrameConstructor::CreateGeneratedContentItem(nsFrameConstructorState& aStat aPseudoElement == CSSPseudoElementType::after, "unexpected aPseudoElement"); - // XXXbz is this ever true? - if (!aParentContent->IsElement()) { - NS_ERROR("Bogus generated content parent"); - return; - } + MOZ_ASSERT(aParentContent->IsElement()); StyleSetHandle styleSet = mPresShell->StyleSet(); @@ -1864,6 +1860,13 @@ nsCSSFrameConstructor::CreateGeneratedContentItem(nsFrameConstructorState& aStat nsresult rv = NS_NewXMLElement(getter_AddRefs(container), nodeInfo.forget()); if (NS_FAILED(rv)) return; + + // Cleared when the pseudo is unbound from the tree, so no need to store a + // strong reference, nor a destructor. + nsIAtom* property = isBefore + ? nsGkAtoms::beforePseudoProperty : nsGkAtoms::afterPseudoProperty; + aParentContent->SetProperty(property, container.get()); + container->SetIsNativeAnonymousRoot(); container->SetPseudoElementType(aPseudoElement); @@ -6079,6 +6082,9 @@ AddGenConPseudoToFrame(nsIFrame* aOwnerFrame, nsIContent* aContent) NS_ASSERTION(nsLayoutUtils::IsFirstContinuationOrIBSplitSibling(aOwnerFrame), "property should only be set on first continuation/ib-sibling"); + // FIXME(emilio): Remove this property, and use the frame of the generated + // content itself to tear the content down? It should be quite simpler. + nsIFrame::ContentArray* value = aOwnerFrame->GetProperty(nsIFrame::GenConProperty()); if (!value) { @@ -6340,7 +6346,7 @@ AdjustAppendParentForAfterContent(nsFrameManager* aFrameManager, // frames to find the first one that is either a ::after frame for an // ancestor of aChild or a frame that is for a node later in the // document than aChild and return that in aAfterFrame. - if (aParentFrame->GetGenConPseudos() || + if (aParentFrame->GetProperty(nsIFrame::GenConProperty()) || nsLayoutUtils::HasPseudoStyle(aContainer, aParentFrame->StyleContext(), CSSPseudoElementType::after, aParentFrame->PresContext()) || @@ -6669,9 +6675,8 @@ nsCSSFrameConstructor::FindFrameForContentSibling(nsIContent* aContent, nsIFrame* sibling = aContent->GetPrimaryFrame(); if (!sibling && GetDisplayContentsStyleFor(aContent)) { // A display:contents node - check if it has a ::before / ::after frame... - sibling = aPrevSibling ? - nsLayoutUtils::GetAfterFrameForContent(aParentFrame, aContent) : - nsLayoutUtils::GetBeforeFrameForContent(aParentFrame, aContent); + sibling = aPrevSibling ? nsLayoutUtils::GetAfterFrame(aContent) + : nsLayoutUtils::GetBeforeFrame(aContent); if (!sibling) { // ... then recurse into children ... const bool forward = !aPrevSibling; @@ -6682,9 +6687,8 @@ nsCSSFrameConstructor::FindFrameForContentSibling(nsIContent* aContent, } if (!sibling) { // ... then ::after / ::before on the opposite end. - sibling = aPrevSibling ? - nsLayoutUtils::GetBeforeFrameForContent(aParentFrame, aContent) : - nsLayoutUtils::GetAfterFrameForContent(aParentFrame, aContent); + sibling = aPrevSibling ? nsLayoutUtils::GetAfterFrame(aContent) + : nsLayoutUtils::GetBeforeFrame(aContent); } if (!sibling) { return nullptr; @@ -8291,26 +8295,23 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, MOZ_ASSERT(!childFrame || !GetDisplayContentsStyleFor(aChild), "display:contents nodes shouldn't have a frame"); if (!childFrame && GetDisplayContentsStyleFor(aChild)) { - nsIFrame* ancestorFrame = nullptr; nsIContent* ancestor = aContainer; - for (; ancestor; ancestor = ancestor->GetParent()) { - ancestorFrame = ancestor->GetPrimaryFrame(); - if (ancestorFrame) { - break; - } + while (!ancestor->GetPrimaryFrame()) { + // FIXME(emilio): Should this use the flattened tree parent instead? + ancestor = ancestor->GetParent(); + MOZ_ASSERT(ancestor, "we can't have a display: contents subtree root!"); } - if (ancestorFrame) { - nsIFrame* contentInsertion = ancestorFrame->GetContentInsertionFrame(); - if (ancestorFrame->GetGenConPseudos() || - (contentInsertion && contentInsertion->GetGenConPseudos())) { - *aDidReconstruct = true; - LAYOUT_PHASE_TEMP_EXIT(); - // XXXmats Can we recreate frames only for the ::after/::before content? - // XXX Perhaps even only those that belong to the aChild sub-tree? - RecreateFramesForContent(ancestor, false, aFlags, aDestroyedFramesFor); - LAYOUT_PHASE_TEMP_REENTER(); - return; - } + + nsIFrame* ancestorFrame = ancestor->GetPrimaryFrame(); + if (ancestorFrame->GetProperty(nsIFrame::GenConProperty())) { + *aDidReconstruct = true; + LAYOUT_PHASE_TEMP_EXIT(); + + // XXXmats Can we recreate frames only for the ::after/::before content? + // XXX Perhaps even only those that belong to the aChild sub-tree? + RecreateFramesForContent(ancestor, false, aFlags, aDestroyedFramesFor); + LAYOUT_PHASE_TEMP_REENTER(); + return; } FlattenedChildIterator iter(aChild); diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 21d20c69fa..de34822460 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -1486,90 +1486,38 @@ nsLayoutUtils::GetChildListNameFor(nsIFrame* aChildFrame) return id; } -/*static*/ nsIFrame* -nsLayoutUtils::GetBeforeFrameForContent(nsIFrame* aFrame, - const nsIContent* aContent) +static Element* +GetPseudo(const nsIContent* aContent, nsIAtom* aPseudoProperty) { - // We need to call GetGenConPseudos() on the first continuation/ib-split. - // Find it, for symmetry with GetAfterFrameForContent. - nsContainerFrame* genConParentFrame = - FirstContinuationOrIBSplitSibling(aFrame)->GetContentInsertionFrame(); - if (!genConParentFrame) { - return nullptr; - } - nsTArray* prop = genConParentFrame->GetGenConPseudos(); - if (prop) { - const nsTArray& pseudos(*prop); - for (uint32_t i = 0; i < pseudos.Length(); ++i) { - if (pseudos[i]->GetParent() == aContent && - pseudos[i]->NodeInfo()->NameAtom() == nsGkAtoms::mozgeneratedcontentbefore) { - return pseudos[i]->GetPrimaryFrame(); - } - } - } - // If the first child frame is a pseudo-frame, then try that. - // Note that the frame we create for the generated content is also a - // pseudo-frame and so don't drill down in that case. - nsIFrame* childFrame = genConParentFrame->PrincipalChildList().FirstChild(); - if (childFrame && - childFrame->IsPseudoFrame(aContent) && - !childFrame->IsGeneratedContentFrame()) { - return GetBeforeFrameForContent(childFrame, aContent); - } - return nullptr; + MOZ_ASSERT(aPseudoProperty == nsGkAtoms::beforePseudoProperty || + aPseudoProperty == nsGkAtoms::afterPseudoProperty); + return static_cast(aContent->GetProperty(aPseudoProperty)); +} + +/*static*/ Element* +nsLayoutUtils::GetBeforePseudo(const nsIContent* aContent) +{ + return GetPseudo(aContent, nsGkAtoms::beforePseudoProperty); } /*static*/ nsIFrame* -nsLayoutUtils::GetBeforeFrame(nsIFrame* aFrame) +nsLayoutUtils::GetBeforeFrame(const nsIContent* aContent) { - return GetBeforeFrameForContent(aFrame, aFrame->GetContent()); + Element* pseudo = GetBeforePseudo(aContent); + return pseudo ? pseudo->GetPrimaryFrame() : nullptr; +} + +/*static*/ Element* +nsLayoutUtils::GetAfterPseudo(const nsIContent* aContent) +{ + return GetPseudo(aContent, nsGkAtoms::afterPseudoProperty); } /*static*/ nsIFrame* -nsLayoutUtils::GetAfterFrameForContent(nsIFrame* aFrame, - const nsIContent* aContent) +nsLayoutUtils::GetAfterFrame(const nsIContent* aContent) { - // We need to call GetGenConPseudos() on the first continuation, - // but callers are likely to pass the last. - nsContainerFrame* genConParentFrame = - FirstContinuationOrIBSplitSibling(aFrame)->GetContentInsertionFrame(); - if (!genConParentFrame) { - return nullptr; - } - nsTArray* prop = genConParentFrame->GetGenConPseudos(); - if (prop) { - const nsTArray& pseudos(*prop); - for (uint32_t i = 0; i < pseudos.Length(); ++i) { - if (pseudos[i]->GetParent() == aContent && - pseudos[i]->NodeInfo()->NameAtom() == nsGkAtoms::mozgeneratedcontentafter) { - return pseudos[i]->GetPrimaryFrame(); - } - } - } - // If the last child frame is a pseudo-frame, then try that. - // Note that the frame we create for the generated content is also a - // pseudo-frame and so don't drill down in that case. - genConParentFrame = aFrame->GetContentInsertionFrame(); - if (!genConParentFrame) { - return nullptr; - } - nsIFrame* lastParentContinuation = - LastContinuationWithChild(static_cast( - LastContinuationOrIBSplitSibling(genConParentFrame))); - nsIFrame* childFrame = - lastParentContinuation->GetChildList(nsIFrame::kPrincipalList).LastChild(); - if (childFrame && - childFrame->IsPseudoFrame(aContent) && - !childFrame->IsGeneratedContentFrame()) { - return GetAfterFrameForContent(childFrame->FirstContinuation(), aContent); - } - return nullptr; -} - -/*static*/ nsIFrame* -nsLayoutUtils::GetAfterFrame(nsIFrame* aFrame) -{ - return GetAfterFrameForContent(aFrame, aFrame->GetContent()); + Element* pseudo = GetAfterPseudo(aContent); + return pseudo ? pseudo->GetPrimaryFrame() : nullptr; } // static diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index bba1f32652..9757cb7999 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -276,50 +276,26 @@ public: static mozilla::layout::FrameChildListID GetChildListNameFor(nsIFrame* aChildFrame); /** - * GetBeforeFrameForContent returns the ::before frame for aContent, if - * one exists. This is typically O(1). The frame passed in must be - * the first-in-flow. - * - * @param aGenConParentFrame an ancestor of the ::before frame - * @param aContent the content whose ::before is wanted - * @return the ::before frame or nullptr if there isn't one + * Returns the ::before pseudo-element for aContent, if any. */ - static nsIFrame* GetBeforeFrameForContent(nsIFrame* aGenConParentFrame, - const nsIContent* aContent); + static mozilla::dom::Element* GetBeforePseudo(const nsIContent* aContent); /** - * GetBeforeFrame returns the outermost ::before frame of the given frame, if - * one exists. This is typically O(1). The frame passed in must be - * the first-in-flow. - * - * @param aFrame the frame whose ::before is wanted - * @return the :before frame or nullptr if there isn't one + * Returns the frame corresponding to the ::before pseudo-element for + * aContent, if any. */ - static nsIFrame* GetBeforeFrame(nsIFrame* aFrame); + static nsIFrame* GetBeforeFrame(const nsIContent* aContent); /** - * GetAfterFrameForContent returns the ::after frame for aContent, if one - * exists. This will walk the in-flow chain of aGenConParentFrame to the - * last-in-flow if needed. This function is typically O(N) in the number - * of child frames, following in-flows, etc. - * - * @param aGenConParentFrame an ancestor of the ::after frame - * @param aContent the content whose ::after is wanted - * @return the ::after frame or nullptr if there isn't one + * Returns the ::after pseudo-element for aContent, if any. */ - static nsIFrame* GetAfterFrameForContent(nsIFrame* aGenConParentFrame, - const nsIContent* aContent); + static mozilla::dom::Element* GetAfterPseudo(const nsIContent* aContent); /** - * GetAfterFrame returns the outermost ::after frame of the given frame, if one - * exists. This will walk the in-flow chain to the last-in-flow if - * needed. This function is typically O(N) in the number of child - * frames, following in-flows, etc. - * - * @param aFrame the frame whose ::after is wanted - * @return the :after frame or nullptr if there isn't one + * Returns the frame corresponding to the ::after pseudo-element for aContent, + * if any. */ - static nsIFrame* GetAfterFrame(nsIFrame* aFrame); + static nsIFrame* GetAfterFrame(const nsIContent* aContent); /** * Given a frame, search up the frame tree until we find an diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp index a498205e70..1035701f41 100644 --- a/layout/generic/nsFrame.cpp +++ b/layout/generic/nsFrame.cpp @@ -9995,19 +9995,16 @@ nsIFrame::IsPseudoStackingContextFromStyle() { Element* nsIFrame::GetPseudoElement(CSSPseudoElementType aType) { - nsIFrame* frame = nullptr; - - if (aType == CSSPseudoElementType::before) { - frame = nsLayoutUtils::GetBeforeFrame(this); - } else if (aType == CSSPseudoElementType::after) { - frame = nsLayoutUtils::GetAfterFrame(this); + if (!mContent) { + return nullptr; } - if (frame) { - nsIContent* content = frame->GetContent(); - if (content->IsElement()) { - return content->AsElement(); - } + if (aType == CSSPseudoElementType::before) { + return nsLayoutUtils::GetBeforePseudo(mContent); + } + + if (aType == CSSPseudoElementType::after) { + return nsLayoutUtils::GetAfterPseudo(mContent); } return nullptr; diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index 3d30f0aed2..67cd727c4f 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -1062,10 +1062,6 @@ public: return GetBidiData().embeddingLevel; } - nsTArray* GetGenConPseudos() { - return GetProperty(GenConProperty()); - } - /** * Return the distance between the border edge of the frame and the * margin edge of the frame. Like GetRect(), returns the dimensions diff --git a/layout/style/nsComputedDOMStyle.cpp b/layout/style/nsComputedDOMStyle.cpp index 102e0df18c..20e5651bd4 100644 --- a/layout/style/nsComputedDOMStyle.cpp +++ b/layout/style/nsComputedDOMStyle.cpp @@ -638,6 +638,34 @@ nsComputedDOMStyle::SetFrameStyleContext(nsStyleContext* aContext) mStyleContext = aContext; } +/** + * The following function checks whether we need to explicitly resolve the style + * again, even though we have a style context coming from the frame. + * + * This basically checks whether the style is or may be under a ::first-line or + * ::first-letter frame, in which case we can't return the frame style, and we + * need to resolve it. See bug 505515. + */ +static bool +MustReresolveStyle(const nsStyleContext* aContext) +{ + MOZ_ASSERT(aContext); + + if (aContext->HasPseudoElementData()) { + if (!aContext->GetPseudo() || + aContext->StyleSource().IsServoComputedValues()) { + // TODO(emilio): When ::first-line is supported in Servo, we may want to + // fix this to avoid re-resolving pseudo-element styles. + return true; + } + + return aContext->GetParent() && + aContext->GetParent()->HasPseudoElementData(); + } + + return false; +} + void nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) { @@ -690,9 +718,20 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) // XXX the !mElement->IsHTMLElement(nsGkAtoms::area) // check is needed due to bug 135040 (to avoid using // mPrimaryFrame). Remove it once that's fixed. - if (!mPseudo && mStyleType == eAll && - !mElement->IsHTMLElement(nsGkAtoms::area)) { - mOuterFrame = mElement->GetPrimaryFrame(); + if (mStyleType == eAll && !mElement->IsHTMLElement(nsGkAtoms::area)) { + mOuterFrame = nullptr; + + if (!mPseudo) { + mOuterFrame = mElement->GetPrimaryFrame(); + } else if (mPseudo == nsCSSPseudoElements::before || + mPseudo == nsCSSPseudoElements::after) { + nsIAtom* property = mPseudo == nsCSSPseudoElements::before + ? nsGkAtoms::beforePseudoProperty + : nsGkAtoms::afterPseudoProperty; + + auto* pseudo = static_cast(mElement->GetProperty(property)); + mOuterFrame = pseudo ? pseudo->GetPrimaryFrame() : nullptr; + } mInnerFrame = mOuterFrame; if (mOuterFrame) { nsIAtom* type = mOuterFrame->GetType(); @@ -711,7 +750,7 @@ nsComputedDOMStyle::UpdateCurrentStyleSources(bool aNeedsLayoutFlush) } } - if (!mStyleContext || mStyleContext->HasPseudoElementData()) { + if (!mStyleContext || MustReresolveStyle(mStyleContext)) { #ifdef DEBUG if (mStyleContext) { // We want to check that going through this path because of From 34d576d55d83a42f7fab6c0555631da781c320e5 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:25:20 -0400 Subject: [PATCH 28/97] Bug 1360157 - Assert that a display: contents child always has a parent Tag #1375 --- layout/base/nsCSSFrameConstructor.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 866fa148bf..bee9ed6299 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -8296,6 +8296,7 @@ nsCSSFrameConstructor::ContentRemoved(nsIContent* aContainer, "display:contents nodes shouldn't have a frame"); if (!childFrame && GetDisplayContentsStyleFor(aChild)) { nsIContent* ancestor = aContainer; + MOZ_ASSERT(ancestor, "display: contents on the root?"); while (!ancestor->GetPrimaryFrame()) { // FIXME(emilio): Should this use the flattened tree parent instead? ancestor = ancestor->GetParent(); From 658bd351a07c5203e4d8efc3026f2acf9cb04d0c Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:26:13 -0400 Subject: [PATCH 29/97] Bug 1361086 - Simplify UndisplayedMap::GetListFor/GetOrCreateListFor to take nsIContent* rather than nsIContent** Tag #1375 --- layout/base/nsFrameManager.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/layout/base/nsFrameManager.cpp b/layout/base/nsFrameManager.cpp index 6bcc715b91..83bcd61ec0 100644 --- a/layout/base/nsFrameManager.cpp +++ b/layout/base/nsFrameManager.cpp @@ -116,8 +116,8 @@ public: void Clear(); protected: - LinkedList* GetListFor(nsIContent** aParentContent); - LinkedList* GetOrCreateListFor(nsIContent** aParentContent); + LinkedList* GetListFor(nsIContent* aParentContent); + LinkedList* GetOrCreateListFor(nsIContent* aParentContent); void AppendNodeFor(UndisplayedNode* aNode, nsIContent* aParentContent); /** * Get the applicable parent for the map lookup. This is almost always the @@ -696,12 +696,12 @@ nsFrameManagerBase::UndisplayedMap::GetApplicableParent(nsIContent* aParent) } LinkedList* -nsFrameManagerBase::UndisplayedMap::GetListFor(nsIContent** aParent) +nsFrameManagerBase::UndisplayedMap::GetListFor(nsIContent* aParent) { - *aParent = GetApplicableParent(*aParent); + aParent = GetApplicableParent(aParent); LinkedList* list; - if (Get(*aParent, &list)) { + if (Get(aParent, &list)) { return list; } @@ -709,17 +709,17 @@ nsFrameManagerBase::UndisplayedMap::GetListFor(nsIContent** aParent) } LinkedList* -nsFrameManagerBase::UndisplayedMap::GetOrCreateListFor(nsIContent** aParent) +nsFrameManagerBase::UndisplayedMap::GetOrCreateListFor(nsIContent* aParent) { - *aParent = GetApplicableParent(*aParent); - return LookupOrAdd(*aParent); + aParent = GetApplicableParent(aParent); + return LookupOrAdd(aParent); } UndisplayedNode* nsFrameManagerBase::UndisplayedMap::GetFirstNode(nsIContent* aParentContent) { - auto* list = GetListFor(&aParentContent); + auto* list = GetListFor(aParentContent); return list ? list->getFirst() : nullptr; } @@ -727,7 +727,7 @@ void nsFrameManagerBase::UndisplayedMap::AppendNodeFor(UndisplayedNode* aNode, nsIContent* aParentContent) { - LinkedList* list = GetOrCreateListFor(&aParentContent); + LinkedList* list = GetOrCreateListFor(aParentContent); #ifdef DEBUG for (UndisplayedNode* node = list->getFirst(); node; node = node->getNext()) { @@ -755,7 +755,7 @@ nsFrameManagerBase::UndisplayedMap::RemoveNodeFor(nsIContent* aParentContent, UndisplayedNode* aNode) { #ifdef DEBUG - auto list = GetListFor(&aParentContent); + auto list = GetListFor(aParentContent); MOZ_ASSERT(list, "content not in map"); aNode->removeFrom(*list); #else From 1f233fa14bafe7db1bd89f683c5311075e6c65c6 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Fri, 17 Apr 2020 05:26:58 -0400 Subject: [PATCH 30/97] Bug 1363481 - Add the old attribute value as a parameter to Element::AfterSetAttr Tag #1375 --- dom/base/Element.cpp | 78 ++++++++++++++++++-------- dom/base/Element.h | 74 ++++++++++++++++++------ dom/base/nsAttrAndChildArray.cpp | 21 ++++--- dom/base/nsAttrAndChildArray.h | 18 ++++-- dom/base/nsMappedAttributeElement.cpp | 14 +++-- dom/base/nsMappedAttributeElement.h | 9 +-- dom/base/nsMappedAttributes.cpp | 7 ++- dom/base/nsMappedAttributes.h | 3 +- dom/base/nsStyledElement.cpp | 12 ++-- dom/html/HTMLBodyElement.cpp | 5 +- dom/html/HTMLBodyElement.h | 4 +- dom/html/HTMLButtonElement.cpp | 6 +- dom/html/HTMLButtonElement.h | 6 +- dom/html/HTMLFieldSetElement.cpp | 5 +- dom/html/HTMLFieldSetElement.h | 4 +- dom/html/HTMLFormElement.cpp | 6 +- dom/html/HTMLFormElement.h | 4 +- dom/html/HTMLIFrameElement.cpp | 4 +- dom/html/HTMLIFrameElement.h | 1 + dom/html/HTMLImageElement.cpp | 5 +- dom/html/HTMLImageElement.h | 4 +- dom/html/HTMLInputElement.cpp | 6 +- dom/html/HTMLInputElement.h | 4 +- dom/html/HTMLLinkElement.cpp | 5 +- dom/html/HTMLLinkElement.h | 1 + dom/html/HTMLMediaElement.cpp | 5 +- dom/html/HTMLMediaElement.h | 1 + dom/html/HTMLMenuItemElement.cpp | 5 +- dom/html/HTMLMenuItemElement.h | 4 +- dom/html/HTMLMetaElement.cpp | 5 +- dom/html/HTMLMetaElement.h | 4 +- dom/html/HTMLOptGroupElement.cpp | 5 +- dom/html/HTMLOptGroupElement.h | 4 +- dom/html/HTMLOptionElement.cpp | 5 +- dom/html/HTMLOptionElement.h | 4 +- dom/html/HTMLScriptElement.cpp | 5 +- dom/html/HTMLScriptElement.h | 4 +- dom/html/HTMLSelectElement.cpp | 6 +- dom/html/HTMLSelectElement.h | 4 +- dom/html/HTMLSourceElement.cpp | 5 +- dom/html/HTMLSourceElement.h | 1 + dom/html/HTMLStyleElement.cpp | 5 +- dom/html/HTMLStyleElement.h | 1 + dom/html/HTMLTableElement.cpp | 8 ++- dom/html/HTMLTableElement.h | 4 +- dom/html/HTMLTextAreaElement.cpp | 5 +- dom/html/HTMLTextAreaElement.h | 4 +- dom/html/nsGenericHTMLElement.cpp | 10 ++-- dom/html/nsGenericHTMLElement.h | 8 ++- dom/html/nsGenericHTMLFrameElement.cpp | 4 +- dom/html/nsGenericHTMLFrameElement.h | 1 + dom/svg/SVGAnimationElement.cpp | 5 +- dom/svg/SVGAnimationElement.h | 4 +- dom/svg/SVGFEImageElement.cpp | 5 +- dom/svg/SVGFEImageElement.h | 4 +- dom/svg/SVGImageElement.cpp | 5 +- dom/svg/SVGImageElement.h | 4 +- dom/svg/SVGScriptElement.cpp | 5 +- dom/svg/SVGScriptElement.h | 4 +- dom/svg/nsSVGElement.cpp | 19 +++++-- dom/svg/nsSVGElement.h | 4 +- dom/svg/nsSVGPathGeometryElement.cpp | 5 +- dom/svg/nsSVGPathGeometryElement.h | 4 +- dom/xul/nsXULElement.cpp | 18 ++++-- dom/xul/nsXULElement.h | 4 +- 65 files changed, 352 insertions(+), 161 deletions(-) diff --git a/dom/base/Element.cpp b/dom/base/Element.cpp index 7c5029e2e2..11ad042a0e 100644 --- a/dom/base/Element.cpp +++ b/dom/base/Element.cpp @@ -2382,7 +2382,8 @@ Element::MaybeCheckSameAttrVal(int32_t aNamespaceID, bool aNotify, nsAttrValue& aOldValue, uint8_t* aModType, - bool* aHasListeners) + bool* aHasListeners, + bool* aOldValueSet) { bool modification = false; *aHasListeners = aNotify && @@ -2390,6 +2391,8 @@ Element::MaybeCheckSameAttrVal(int32_t aNamespaceID, NS_EVENT_BITS_MUTATION_ATTRMODIFIED, this); + *aOldValueSet = false; + // If we have no listeners and aNotify is false, we are almost certainly // coming from the content sink and will almost certainly have no previous // value. Even if we do, setting the value is cheap when we have no @@ -2413,6 +2416,7 @@ Element::MaybeCheckSameAttrVal(int32_t aNamespaceID, // We have to serialize the value anyway in order to create the // mutation event so there's no cost in doing it now. aOldValue.SetToSerialized(*info.mValue); + *aOldValueSet = true; } bool valueMatches = aValue.EqualsAsStrings(*info.mValue); if (valueMatches && aPrefix == info.mName->GetPrefix()) { @@ -2432,10 +2436,12 @@ Element::OnlyNotifySameValueSet(int32_t aNamespaceID, nsIAtom* aName, nsIAtom* aPrefix, const nsAttrValueOrString& aValue, bool aNotify, nsAttrValue& aOldValue, - uint8_t* aModType, bool* aHasListeners) + uint8_t* aModType, bool* aHasListeners, + bool* aOldValueSet) { if (!MaybeCheckSameAttrVal(aNamespaceID, aName, aPrefix, aValue, aNotify, - aOldValue, aModType, aHasListeners)) { + aOldValue, aModType, aHasListeners, + aOldValueSet)) { return false; } @@ -2466,9 +2472,10 @@ Element::SetAttr(int32_t aNamespaceID, nsIAtom* aName, // OnlyNotifySameValueSet call. nsAttrValueOrString value(aValue); nsAttrValue oldValue; + bool oldValueSet; if (OnlyNotifySameValueSet(aNamespaceID, aName, aPrefix, value, aNotify, - oldValue, &modType, &hasListeners)) { + oldValue, &modType, &hasListeners, &oldValueSet)) { return NS_OK; } @@ -2501,7 +2508,8 @@ Element::SetAttr(int32_t aNamespaceID, nsIAtom* aName, attrValue.SetTo(aValue); } - return SetAttrAndNotify(aNamespaceID, aName, aPrefix, oldValue, + return SetAttrAndNotify(aNamespaceID, aName, aPrefix, + oldValueSet ? &oldValue : nullptr, attrValue, modType, hasListeners, aNotify, kCallAfterSetAttr, document, updateBatch); } @@ -2526,9 +2534,10 @@ Element::SetParsedAttr(int32_t aNamespaceID, nsIAtom* aName, bool hasListeners; nsAttrValueOrString value(aParsedValue); nsAttrValue oldValue; + bool oldValueSet; if (OnlyNotifySameValueSet(aNamespaceID, aName, aPrefix, value, aNotify, - oldValue, &modType, &hasListeners)) { + oldValue, &modType, &hasListeners, &oldValueSet)) { return NS_OK; } @@ -2542,7 +2551,8 @@ Element::SetParsedAttr(int32_t aNamespaceID, nsIAtom* aName, nsIDocument* document = GetComposedDoc(); mozAutoDocUpdate updateBatch(document, UPDATE_CONTENT_MODEL, aNotify); - return SetAttrAndNotify(aNamespaceID, aName, aPrefix, oldValue, + return SetAttrAndNotify(aNamespaceID, aName, aPrefix, + oldValueSet ? &oldValue : nullptr, aParsedValue, modType, hasListeners, aNotify, kCallAfterSetAttr, document, updateBatch); } @@ -2551,7 +2561,7 @@ nsresult Element::SetAttrAndNotify(int32_t aNamespaceID, nsIAtom* aName, nsIAtom* aPrefix, - const nsAttrValue& aOldValue, + const nsAttrValue* aOldValue, nsAttrValue& aParsedValue, uint8_t aModType, bool aFireMutation, @@ -2573,6 +2583,7 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, bool hadValidDir = false; bool hadDirAuto = false; + bool oldValueSet; if (aNamespaceID == kNameSpaceID_None) { if (aName == nsGkAtoms::dir) { @@ -2583,8 +2594,8 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, // XXXbz Perhaps we should push up the attribute mapping function // stuff to Element? if (!IsAttributeMapped(aName) || - !SetMappedAttribute(aComposedDocument, aName, aParsedValue, &rv)) { - rv = mAttrsAndChildren.SetAndSwapAttr(aName, aParsedValue); + !SetAndSwapMappedAttribute(aComposedDocument, aName, aParsedValue, &oldValueSet, &rv)) { + rv = mAttrsAndChildren.SetAndSwapAttr(aName, aParsedValue, &oldValueSet); } } else { @@ -2593,14 +2604,24 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, aNamespaceID, nsIDOMNode::ATTRIBUTE_NODE); - rv = mAttrsAndChildren.SetAndSwapAttr(ni, aParsedValue); + rv = mAttrsAndChildren.SetAndSwapAttr(ni, aParsedValue, &oldValueSet); } + NS_ENSURE_SUCCESS(rv, rv); // If the old value owns its own data, we know it is OK to keep using it. - const nsAttrValue* oldValue = - aParsedValue.StoresOwnData() ? &aParsedValue : &aOldValue; - - NS_ENSURE_SUCCESS(rv, rv); + // oldValue will be null if there was no previously set value + const nsAttrValue* oldValue; + if (aParsedValue.StoresOwnData()) { + if (oldValueSet) { + oldValue = &aParsedValue; + } else { + oldValue = nullptr; + } + } else { + // No need to conditionally assign null here. If there was no previously + // set value for the attribute, aOldValue will already be null. + oldValue = aOldValue; + } if (aComposedDocument || HasFlag(NODE_FORCE_XBL_BINDINGS)) { RefPtr binding = GetXBLBinding(); @@ -2618,7 +2639,14 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, MOZ_ASSERT(data->mState == CustomElementData::State::eCustom, "AttributeChanged callback should fire only if " "custom element state is custom"); - nsCOMPtr oldValueAtom = oldValue->GetAsAtom(); + nsCOMPtr oldValueAtom; + if (oldValue) { + oldValueAtom = oldValue->GetAsAtom(); + } else { + // If there is no old value, get the value of the uninitialized attribute + // that was swapped with aParsedValue. + oldValueAtom = aParsedValue.GetAsAtom(); + } nsCOMPtr newValueAtom = valueForAfterSetAttr.GetAsAtom(); nsAutoString ns; nsContentUtils::NameSpaceManager()->GetNameSpaceURI(aNamespaceID, ns); @@ -2638,7 +2666,8 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, } if (aCallAfterSetAttr) { - rv = AfterSetAttr(aNamespaceID, aName, &valueForAfterSetAttr, aNotify); + rv = AfterSetAttr(aNamespaceID, aName, &valueForAfterSetAttr, oldValue, + aNotify); NS_ENSURE_SUCCESS(rv, rv); if (aNamespaceID == kNameSpaceID_None && aName == nsGkAtoms::dir) { @@ -2654,7 +2683,7 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, // Callers only compute aOldValue under certain conditions which may not // be triggered by all nsIMutationObservers. nsNodeUtils::AttributeChanged(this, aNamespaceID, aName, aModType, - oldValue == &aParsedValue ? &aParsedValue : nullptr); + aParsedValue.StoresOwnData() ? &aParsedValue : nullptr); } if (aFireMutation) { @@ -2672,7 +2701,7 @@ Element::SetAttrAndNotify(int32_t aNamespaceID, if (!newValue.IsEmpty()) { mutation.mNewAttrValue = NS_Atomize(newValue); } - if (!oldValue->IsEmptyString()) { + if (oldValue && !oldValue->IsEmptyString()) { mutation.mPrevAttrValue = oldValue->GetAsAtom(); } mutation.mAttrChange = aModType; @@ -2715,10 +2744,11 @@ Element::ParseAttribute(int32_t aNamespaceID, } bool -Element::SetMappedAttribute(nsIDocument* aDocument, - nsIAtom* aName, - nsAttrValue& aValue, - nsresult* aRetval) +Element::SetAndSwapMappedAttribute(nsIDocument* aDocument, + nsIAtom* aName, + nsAttrValue& aValue, + bool* aValueWasSet, + nsresult* aRetval) { *aRetval = NS_OK; return false; @@ -2886,7 +2916,7 @@ Element::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aName, } } - rv = AfterSetAttr(aNameSpaceID, aName, nullptr, aNotify); + rv = AfterSetAttr(aNameSpaceID, aName, nullptr, &oldValue, aNotify); NS_ENSURE_SUCCESS(rv, rv); UpdateState(aNotify); diff --git a/dom/base/Element.h b/dom/base/Element.h index 6b55cf8cd7..6f60c9ad0d 100644 --- a/dom/base/Element.h +++ b/dom/base/Element.h @@ -537,25 +537,50 @@ public: * values will not actually be compared if we aren't notifying and we don't * have mutation listeners (in which case it's cheap to just return false * and let the caller go ahead and set the value). - * @param aOldValue Set to the old value of the attribute, but only if there - * are event listeners. If set, the type of aOldValue will be either + * @param aOldValue [out] Set to the old value of the attribute, but only if + * there are event listeners. If set, the type of aOldValue will be either * nsAttrValue::eString or nsAttrValue::eAtom. - * @param aModType Set to nsIDOMMutationEvent::MODIFICATION or to + * @param aModType [out] Set to nsIDOMMutationEvent::MODIFICATION or to * nsIDOMMutationEvent::ADDITION, but only if this helper returns true - * @param aHasListeners Set to true if there are mutation event listeners - * listening for NS_EVENT_BITS_MUTATION_ATTRMODIFIED + * @param aHasListeners [out] Set to true if there are mutation event + * listeners listening for NS_EVENT_BITS_MUTATION_ATTRMODIFIED + * @param aOldValueSet [out] Indicates whether an old attribute value has been + * stored in aOldValue. The bool will be set to true if a value was stored. */ bool MaybeCheckSameAttrVal(int32_t aNamespaceID, nsIAtom* aName, nsIAtom* aPrefix, const nsAttrValueOrString& aValue, bool aNotify, nsAttrValue& aOldValue, - uint8_t* aModType, bool* aHasListeners); + uint8_t* aModType, bool* aHasListeners, + bool* aOldValueSet); + + /** + * Notifies mutation listeners if aNotify is true, there are mutation + * listeners, and the attribute value is changing. + * + * @param aNamespaceID The namespace of the attribute + * @param aName The local name of the attribute + * @param aPrefix The prefix of the attribute + * @param aValue The value that the attribute is being changed to + * @param aNotify If true, mutation listeners will be notified if they exist + * and the attribute value is changing + * @param aOldValue [out] Set to the old value of the attribute, but only if + * there are event listeners. If set, the type of aOldValue will be either + * nsAttrValue::eString or nsAttrValue::eAtom. + * @param aModType [out] Set to nsIDOMMutationEvent::MODIFICATION or to + * nsIDOMMutationEvent::ADDITION, but only if this helper returns true + * @param aHasListeners [out] Set to true if there are mutation event + * listeners listening for NS_EVENT_BITS_MUTATION_ATTRMODIFIED + * @param aOldValueSet [out] Indicates whether an old attribute value has been + * stored in aOldValue. The bool will be set to true if a value was stored. + */ bool OnlyNotifySameValueSet(int32_t aNamespaceID, nsIAtom* aName, nsIAtom* aPrefix, const nsAttrValueOrString& aValue, bool aNotify, nsAttrValue& aOldValue, - uint8_t* aModType, bool* aHasListeners); + uint8_t* aModType, bool* aHasListeners, + bool* aOldValueSet); virtual nsresult SetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsIAtom* aPrefix, const nsAString& aValue, bool aNotify) override; @@ -1261,7 +1286,10 @@ protected: * its current value) is !StoresOwnData() --- in which * case the current value is probably already useless. * If the current value is StoresOwnData() (or absent), - * aOldValue will not be used. + * aOldValue will not be used. aOldValue will only be set + * in certain circumstances (there are mutation + * listeners, element is a custom element, attribute was + * not previously unset). Otherwise it will be null. * @param aParsedValue parsed new value of attribute. Replaced by the * old value of the attribute. This old value is only * useful if either it or the new value is StoresOwnData. @@ -1275,7 +1303,7 @@ protected: nsresult SetAttrAndNotify(int32_t aNamespaceID, nsIAtom* aName, nsIAtom* aPrefix, - const nsAttrValue& aOldValue, + const nsAttrValue* aOldValue, nsAttrValue& aParsedValue, uint8_t aModType, bool aFireMutation, @@ -1320,14 +1348,21 @@ protected: * * @param aDocument the current document of this node (an optimization) * @param aName the name of the attribute - * @param aValue the nsAttrValue to set + * @param aValue the nsAttrValue to set. Will be swapped with the existing + * value of the attribute if the attribute already exists. + * @param [out] aValueWasSet If the attribute was not set previously, + * aValue will be swapped with an empty attribute + * and aValueWasSet will be set to false. Otherwise, + * aValueWasSet will be set to true and aValue will + * contain the previous value set. * @param [out] aRetval the nsresult status of the operation, if any. * @return true if the setting was attempted, false otherwise. */ - virtual bool SetMappedAttribute(nsIDocument* aDocument, - nsIAtom* aName, - nsAttrValue& aValue, - nsresult* aRetval); + virtual bool SetAndSwapMappedAttribute(nsIDocument* aDocument, + nsIAtom* aName, + nsAttrValue& aValue, + bool* aValueWasSet, + nsresult* aRetval); /** * Hook that is called by Element::SetAttr to allow subclasses to @@ -1355,19 +1390,24 @@ protected: /** * Hook that is called by Element::SetAttr to allow subclasses to * deal with attribute sets. This will only be called after we have called - * SetAndTakeAttr and AttributeChanged (that is, after we have actually set - * the attr). It will always be called under a scriptblocker. + * SetAndSwapAttr (that is, after we have actually set the attr). It will + * always be called under a scriptblocker. * * @param aNamespaceID the namespace of the attr being set * @param aName the localname of the attribute being set * @param aValue the value it's being set to. If null, the attr is being * removed. + * @param aOldValue the value that the attribute had previously. If null, + * the attr was not previously set. This argument may not have the + * correct value for SVG elements, or other cases in which the + * attribute value doesn't store its own data * @param aNotify Whether we plan to notify document observers. */ // Note that this is inlined so that when subclasses call it it gets // inlined. Those calls don't go through a vtable. virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { return NS_OK; } diff --git a/dom/base/nsAttrAndChildArray.cpp b/dom/base/nsAttrAndChildArray.cpp index 4daac498ba..f8e4c3f184 100644 --- a/dom/base/nsAttrAndChildArray.cpp +++ b/dom/base/nsAttrAndChildArray.cpp @@ -382,12 +382,15 @@ nsAttrAndChildArray::AttrAt(uint32_t aPos) const } nsresult -nsAttrAndChildArray::SetAndSwapAttr(nsIAtom* aLocalName, nsAttrValue& aValue) +nsAttrAndChildArray::SetAndSwapAttr(nsIAtom* aLocalName, nsAttrValue& aValue, + bool* aHadValue) { + *aHadValue = false; uint32_t i, slotCount = AttrSlotCount(); for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) { if (ATTRS(mImpl)[i].mName.Equals(aLocalName)) { ATTRS(mImpl)[i].mValue.SwapValueWith(aValue); + *aHadValue = true; return NS_OK; } } @@ -407,21 +410,22 @@ nsAttrAndChildArray::SetAndSwapAttr(nsIAtom* aLocalName, nsAttrValue& aValue) } nsresult -nsAttrAndChildArray::SetAndSwapAttr(mozilla::dom::NodeInfo* aName, nsAttrValue& aValue) +nsAttrAndChildArray::SetAndSwapAttr(mozilla::dom::NodeInfo* aName, + nsAttrValue& aValue, bool* aHadValue) { int32_t namespaceID = aName->NamespaceID(); nsIAtom* localName = aName->NameAtom(); if (namespaceID == kNameSpaceID_None) { - return SetAndSwapAttr(localName, aValue); + return SetAndSwapAttr(localName, aValue, aHadValue); } + *aHadValue = false; uint32_t i, slotCount = AttrSlotCount(); for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) { if (ATTRS(mImpl)[i].mName.Equals(localName, namespaceID)) { ATTRS(mImpl)[i].mName.SetTo(aName); - ATTRS(mImpl)[i].mValue.Reset(); ATTRS(mImpl)[i].mValue.SwapValueWith(aValue); - + *aHadValue = true; return NS_OK; } } @@ -576,10 +580,11 @@ nsAttrAndChildArray::IndexOfAttr(nsIAtom* aLocalName, int32_t aNamespaceID) cons } nsresult -nsAttrAndChildArray::SetAndTakeMappedAttr(nsIAtom* aLocalName, +nsAttrAndChildArray::SetAndSwapMappedAttr(nsIAtom* aLocalName, nsAttrValue& aValue, nsMappedAttributeElement* aContent, - nsHTMLStyleSheet* aSheet) + nsHTMLStyleSheet* aSheet, + bool* aHadValue) { bool willAdd = true; if (mImpl && mImpl->mMappedAttrs) { @@ -589,7 +594,7 @@ nsAttrAndChildArray::SetAndTakeMappedAttr(nsIAtom* aLocalName, RefPtr mapped = GetModifiableMapped(aContent, aSheet, willAdd); - mapped->SetAndTakeAttr(aLocalName, aValue); + mapped->SetAndSwapAttr(aLocalName, aValue, aHadValue); return MakeMappedUnique(mapped); } diff --git a/dom/base/nsAttrAndChildArray.h b/dom/base/nsAttrAndChildArray.h index ffbad196c5..0ce51c52eb 100644 --- a/dom/base/nsAttrAndChildArray.h +++ b/dom/base/nsAttrAndChildArray.h @@ -91,8 +91,13 @@ public: nsCaseTreatment aCaseSensitive) const; const nsAttrValue* AttrAt(uint32_t aPos) const; // SetAndSwapAttr swaps the current attribute value with aValue. - nsresult SetAndSwapAttr(nsIAtom* aLocalName, nsAttrValue& aValue); - nsresult SetAndSwapAttr(mozilla::dom::NodeInfo* aName, nsAttrValue& aValue); + // If the attribute was unset, an empty value will be swapped into aValue + // and aHadValue will be set to false. Otherwise, aHadValue will be set to + // true. + nsresult SetAndSwapAttr(nsIAtom* aLocalName, nsAttrValue& aValue, + bool* aHadValue); + nsresult SetAndSwapAttr(mozilla::dom::NodeInfo* aName, nsAttrValue& aValue, + bool* aHadValue); // Remove the attr at position aPos. The value of the attr is placed in // aValue; any value that was already in aValue is destroyed. @@ -110,9 +115,14 @@ public: const nsAttrName* GetExistingAttrNameFromQName(const nsAString& aName) const; int32_t IndexOfAttr(nsIAtom* aLocalName, int32_t aNamespaceID = kNameSpaceID_None) const; - nsresult SetAndTakeMappedAttr(nsIAtom* aLocalName, nsAttrValue& aValue, + // SetAndSwapMappedAttr swaps the current attribute value with aValue. + // If the attribute was unset, an empty value will be swapped into aValue + // and aHadValue will be set to false. Otherwise, aHadValue will be set to + // true. + nsresult SetAndSwapMappedAttr(nsIAtom* aLocalName, nsAttrValue& aValue, nsMappedAttributeElement* aContent, - nsHTMLStyleSheet* aSheet); + nsHTMLStyleSheet* aSheet, + bool* aHadValue); nsresult SetMappedAttrStyleSheet(nsHTMLStyleSheet* aSheet) { if (!mImpl || !mImpl->mMappedAttrs) { return NS_OK; diff --git a/dom/base/nsMappedAttributeElement.cpp b/dom/base/nsMappedAttributeElement.cpp index 1c1f8838fa..d06cbf2bc5 100644 --- a/dom/base/nsMappedAttributeElement.cpp +++ b/dom/base/nsMappedAttributeElement.cpp @@ -15,17 +15,19 @@ nsMappedAttributeElement::WalkContentStyleRules(nsRuleWalker* aRuleWalker) } bool -nsMappedAttributeElement::SetMappedAttribute(nsIDocument* aDocument, - nsIAtom* aName, - nsAttrValue& aValue, - nsresult* aRetval) +nsMappedAttributeElement::SetAndSwapMappedAttribute(nsIDocument* aDocument, + nsIAtom* aName, + nsAttrValue& aValue, + bool* aValueWasSet, + nsresult* aRetval) + { NS_PRECONDITION(aDocument == GetComposedDoc(), "Unexpected document"); nsHTMLStyleSheet* sheet = aDocument ? aDocument->GetAttributeStyleSheet() : nullptr; - *aRetval = mAttrsAndChildren.SetAndTakeMappedAttr(aName, aValue, - this, sheet); + *aRetval = mAttrsAndChildren.SetAndSwapMappedAttr(aName, aValue, + this, sheet, aValueWasSet); return true; } diff --git a/dom/base/nsMappedAttributeElement.h b/dom/base/nsMappedAttributeElement.h index 4668b36a1a..a37af85ca8 100644 --- a/dom/base/nsMappedAttributeElement.h +++ b/dom/base/nsMappedAttributeElement.h @@ -39,10 +39,11 @@ public: nsRuleData* aRuleData); NS_IMETHOD WalkContentStyleRules(nsRuleWalker* aRuleWalker) override; - virtual bool SetMappedAttribute(nsIDocument* aDocument, - nsIAtom* aName, - nsAttrValue& aValue, - nsresult* aRetval) override; + virtual bool SetAndSwapMappedAttribute(nsIDocument* aDocument, + nsIAtom* aName, + nsAttrValue& aValue, + bool* aValueWasSet, + nsresult* aRetval) override; }; #endif // NS_MAPPEDATTRIBUTEELEMENT_H_ diff --git a/dom/base/nsMappedAttributes.cpp b/dom/base/nsMappedAttributes.cpp index 9e80c94df6..05ad423103 100644 --- a/dom/base/nsMappedAttributes.cpp +++ b/dom/base/nsMappedAttributes.cpp @@ -85,15 +85,16 @@ NS_IMPL_ISUPPORTS(nsMappedAttributes, nsIStyleRule) void -nsMappedAttributes::SetAndTakeAttr(nsIAtom* aAttrName, nsAttrValue& aValue) +nsMappedAttributes::SetAndSwapAttr(nsIAtom* aAttrName, nsAttrValue& aValue, + bool* aValueWasSet) { NS_PRECONDITION(aAttrName, "null name"); - + *aValueWasSet = false; uint32_t i; for (i = 0; i < mAttrCount && !Attrs()[i].mName.IsSmaller(aAttrName); ++i) { if (Attrs()[i].mName.Equals(aAttrName)) { - Attrs()[i].mValue.Reset(); Attrs()[i].mValue.SwapValueWith(aValue); + *aValueWasSet = true; return; } } diff --git a/dom/base/nsMappedAttributes.h b/dom/base/nsMappedAttributes.h index f00b888b9e..0c24fd9737 100644 --- a/dom/base/nsMappedAttributes.h +++ b/dom/base/nsMappedAttributes.h @@ -33,7 +33,8 @@ public: NS_DECL_ISUPPORTS - void SetAndTakeAttr(nsIAtom* aAttrName, nsAttrValue& aValue); + void SetAndSwapAttr(nsIAtom* aAttrName, nsAttrValue& aValue, + bool* aValueWasSet); const nsAttrValue* GetAttr(nsIAtom* aAttrName) const; const nsAttrValue* GetAttr(const nsAString& aAttrName) const; diff --git a/dom/base/nsStyledElement.cpp b/dom/base/nsStyledElement.cpp index cdfa564616..e3d8dfe574 100644 --- a/dom/base/nsStyledElement.cpp +++ b/dom/base/nsStyledElement.cpp @@ -57,6 +57,7 @@ nsStyledElement::SetInlineStyleDeclaration(DeclarationBlock* aDeclaration, SetMayHaveStyle(); bool modification = false; nsAttrValue oldValue; + bool oldValueSet = false; bool hasListeners = aNotify && nsContentUtils::HasMutationListeners(this, @@ -76,6 +77,7 @@ nsStyledElement::SetInlineStyleDeclaration(DeclarationBlock* aDeclaration, oldValueStr); if (modification) { oldValue.SetTo(oldValueStr); + oldValueSet = true; } } else if (aNotify && IsInUncomposedDoc()) { @@ -92,9 +94,9 @@ nsStyledElement::SetInlineStyleDeclaration(DeclarationBlock* aDeclaration, nsIDocument* document = GetComposedDoc(); mozAutoDocUpdate updateBatch(document, UPDATE_CONTENT_MODEL, aNotify); return SetAttrAndNotify(kNameSpaceID_None, nsGkAtoms::style, nullptr, - oldValue, attrValue, modType, hasListeners, - aNotify, kDontCallAfterSetAttr, document, - updateBatch); + oldValueSet ? &oldValue : nullptr, attrValue, modType, + hasListeners, aNotify, kDontCallAfterSetAttr, + document, updateBatch); } DeclarationBlock* @@ -145,7 +147,9 @@ nsStyledElement::ReparseStyleAttribute(bool aForceInDataDoc) ParseStyleAttribute(stringValue, attrValue, aForceInDataDoc); // Don't bother going through SetInlineStyleDeclaration; we don't // want to fire off mutation events or document notifications anyway - nsresult rv = mAttrsAndChildren.SetAndSwapAttr(nsGkAtoms::style, attrValue); + bool oldValueSet; + nsresult rv = mAttrsAndChildren.SetAndSwapAttr(nsGkAtoms::style, attrValue, + &oldValueSet); NS_ENSURE_SUCCESS(rv, rv); } diff --git a/dom/html/HTMLBodyElement.cpp b/dom/html/HTMLBodyElement.cpp index f70e203897..112d48dd15 100644 --- a/dom/html/HTMLBodyElement.cpp +++ b/dom/html/HTMLBodyElement.cpp @@ -307,10 +307,11 @@ HTMLBodyElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, nsresult HTMLBodyElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, - bool aNotify) + const nsAttrValue* aOldValue, bool aNotify) { nsresult rv = nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, - aName, aValue, aNotify); + aName, aValue, aOldValue, + aNotify); NS_ENSURE_SUCCESS(rv, rv); // if the last mapped attribute was removed, don't clear the // nsMappedAttributes, our style can still depend on the containing frame element diff --git a/dom/html/HTMLBodyElement.h b/dom/html/HTMLBodyElement.h index ad1f5d8721..6a888c71b7 100644 --- a/dom/html/HTMLBodyElement.h +++ b/dom/html/HTMLBodyElement.h @@ -117,7 +117,9 @@ public: * Called when an attribute has just been changed */ virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; protected: virtual ~HTMLBodyElement(); diff --git a/dom/html/HTMLButtonElement.cpp b/dom/html/HTMLButtonElement.cpp index c26e60ed0f..1a76aac10a 100644 --- a/dom/html/HTMLButtonElement.cpp +++ b/dom/html/HTMLButtonElement.cpp @@ -437,7 +437,8 @@ HTMLButtonElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsresult HTMLButtonElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None) { if (aName == nsGkAtoms::type) { @@ -452,7 +453,8 @@ HTMLButtonElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLFormElementWithState::AfterSetAttr(aNameSpaceID, aName, - aValue, aNotify); + aValue, aOldValue, + aNotify); } NS_IMETHODIMP diff --git a/dom/html/HTMLButtonElement.h b/dom/html/HTMLButtonElement.h index 19c93e4839..8bab9fd48e 100644 --- a/dom/html/HTMLButtonElement.h +++ b/dom/html/HTMLButtonElement.h @@ -86,8 +86,10 @@ public: /** * Called when an attribute has just been changed */ - nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; virtual bool ParseAttribute(int32_t aNamespaceID, nsIAtom* aAttribute, const nsAString& aValue, diff --git a/dom/html/HTMLFieldSetElement.cpp b/dom/html/HTMLFieldSetElement.cpp index c008b9b6d6..f546cad5b4 100644 --- a/dom/html/HTMLFieldSetElement.cpp +++ b/dom/html/HTMLFieldSetElement.cpp @@ -83,7 +83,8 @@ HTMLFieldSetElement::GetEventTargetParent(EventChainPreVisitor& aVisitor) nsresult HTMLFieldSetElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::disabled && nsINode::GetFirstChild()) { @@ -100,7 +101,7 @@ HTMLFieldSetElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLFormElement::AfterSetAttr(aNameSpaceID, aName, - aValue, aNotify); + aValue, aOldValue, aNotify); } // nsIDOMHTMLFieldSetElement diff --git a/dom/html/HTMLFieldSetElement.h b/dom/html/HTMLFieldSetElement.h index 1e9f0214f5..2c0e9cc149 100644 --- a/dom/html/HTMLFieldSetElement.h +++ b/dom/html/HTMLFieldSetElement.h @@ -43,7 +43,9 @@ public: virtual nsresult GetEventTargetParent( EventChainPreVisitor& aVisitor) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; virtual nsresult InsertChildAt(nsIContent* aChild, uint32_t aIndex, bool aNotify) override; diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index bdc05b053c..1fdfb602e1 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -214,7 +214,8 @@ HTMLFormElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsresult HTMLFormElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aName == nsGkAtoms::novalidate && aNameSpaceID == kNameSpaceID_None) { // Update all form elements states because they might be [no longer] @@ -230,7 +231,8 @@ HTMLFormElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } } - return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, aNotify); + return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, + aOldValue, aNotify); } NS_IMPL_STRING_ATTR(HTMLFormElement, AcceptCharset, acceptcharset) diff --git a/dom/html/HTMLFormElement.h b/dom/html/HTMLFormElement.h index e45d4c10d7..6102ea61f1 100644 --- a/dom/html/HTMLFormElement.h +++ b/dom/html/HTMLFormElement.h @@ -114,7 +114,9 @@ public: nsIAtom* aPrefix, const nsAString& aValue, bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; /** * Forget all information about the current submission (and the fact that we diff --git a/dom/html/HTMLIFrameElement.cpp b/dom/html/HTMLIFrameElement.cpp index 0d5a209c04..6ae4ec9ddd 100644 --- a/dom/html/HTMLIFrameElement.cpp +++ b/dom/html/HTMLIFrameElement.cpp @@ -202,7 +202,7 @@ HTMLIFrameElement::SetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsresult HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, - bool aNotify) + const nsAttrValue* aOldValue, bool aNotify) { if (aName == nsGkAtoms::sandbox && aNameSpaceID == kNameSpaceID_None && mFrameLoader) { @@ -212,7 +212,7 @@ HTMLIFrameElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, mFrameLoader->ApplySandboxFlags(GetSandboxFlags()); } return nsGenericHTMLFrameElement::AfterSetAttr(aNameSpaceID, aName, aValue, - aNotify); + aOldValue, aNotify); } nsresult diff --git a/dom/html/HTMLIFrameElement.h b/dom/html/HTMLIFrameElement.h index 5bfda24700..23afaaddd0 100644 --- a/dom/html/HTMLIFrameElement.h +++ b/dom/html/HTMLIFrameElement.h @@ -56,6 +56,7 @@ public: bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) override; virtual nsresult UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttribute, bool aNotify) override; diff --git a/dom/html/HTMLImageElement.cpp b/dom/html/HTMLImageElement.cpp index 5d0cfd15eb..dd43ac36ad 100644 --- a/dom/html/HTMLImageElement.cpp +++ b/dom/html/HTMLImageElement.cpp @@ -391,7 +391,8 @@ HTMLImageElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsresult HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None && mForm && (aName == nsGkAtoms::name || aName == nsGkAtoms::id) && @@ -433,7 +434,7 @@ HTMLImageElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, - aValue, aNotify); + aValue, aOldValue, aNotify); } nsresult diff --git a/dom/html/HTMLImageElement.h b/dom/html/HTMLImageElement.h index d92cd84b81..52480d0a05 100644 --- a/dom/html/HTMLImageElement.h +++ b/dom/html/HTMLImageElement.h @@ -348,7 +348,9 @@ protected: bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; // This is a weak reference that this element and the HTMLFormElement // cooperate in maintaining. diff --git a/dom/html/HTMLInputElement.cpp b/dom/html/HTMLInputElement.cpp index 1d3db145ac..7c2688b7ed 100644 --- a/dom/html/HTMLInputElement.cpp +++ b/dom/html/HTMLInputElement.cpp @@ -1287,7 +1287,8 @@ HTMLInputElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsresult HTMLInputElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None) { // @@ -1437,7 +1438,8 @@ HTMLInputElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLFormElementWithState::AfterSetAttr(aNameSpaceID, aName, - aValue, aNotify); + aValue, aOldValue, + aNotify); } // nsIDOMHTMLInputElement diff --git a/dom/html/HTMLInputElement.h b/dom/html/HTMLInputElement.h index d6a078f0bf..48807f7337 100644 --- a/dom/html/HTMLInputElement.h +++ b/dom/html/HTMLInputElement.h @@ -964,7 +964,9 @@ protected: * Called when an attribute has just been changed */ virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; /** * Dispatch a select event. Returns true if the event was not cancelled. diff --git a/dom/html/HTMLLinkElement.cpp b/dom/html/HTMLLinkElement.cpp index 4c42d310a6..b05b92d7aa 100644 --- a/dom/html/HTMLLinkElement.cpp +++ b/dom/html/HTMLLinkElement.cpp @@ -348,7 +348,8 @@ HTMLLinkElement::BeforeSetAttr(int32_t aNameSpaceID, nsIAtom* aName, nsresult HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { // It's safe to call ResetLinkState here because our new attr value has // already been set or unset. ResetLinkState needs the updated attribute @@ -417,7 +418,7 @@ HTMLLinkElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, - aNotify); + aOldValue, aNotify); } nsresult diff --git a/dom/html/HTMLLinkElement.h b/dom/html/HTMLLinkElement.h index 9c69cfb13c..acac955cb2 100644 --- a/dom/html/HTMLLinkElement.h +++ b/dom/html/HTMLLinkElement.h @@ -66,6 +66,7 @@ public: bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) override; virtual bool IsLink(nsIURI** aURI) const override; virtual already_AddRefed GetHrefURI() const override; diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index bc63eab512..4bcc188d39 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -3774,7 +3774,8 @@ nsresult HTMLMediaElement::UnsetAttr(int32_t aNameSpaceID, nsIAtom* aAttr, nsresult HTMLMediaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::src) { mSrcMediaSource = nullptr; @@ -3796,7 +3797,7 @@ HTMLMediaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, - aValue, aNotify); + aValue, aOldValue, aNotify); } nsresult HTMLMediaElement::BindToTree(nsIDocument* aDocument, nsIContent* aParent, diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index 44c666f951..d81e9f5918 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -146,6 +146,7 @@ public: bool aNotify) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) override; virtual nsresult BindToTree(nsIDocument* aDocument, nsIContent* aParent, diff --git a/dom/html/HTMLMenuItemElement.cpp b/dom/html/HTMLMenuItemElement.cpp index 6a8ad8174c..3245a16b1d 100644 --- a/dom/html/HTMLMenuItemElement.cpp +++ b/dom/html/HTMLMenuItemElement.cpp @@ -380,7 +380,8 @@ HTMLMenuItemElement::GetText(nsAString& aText) nsresult HTMLMenuItemElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None) { if ((aName == nsGkAtoms::radiogroup || aName == nsGkAtoms::type) && @@ -404,7 +405,7 @@ HTMLMenuItemElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, - aNotify); + aOldValue, aNotify); } void diff --git a/dom/html/HTMLMenuItemElement.h b/dom/html/HTMLMenuItemElement.h index 1da63edd05..cb824edcc5 100644 --- a/dom/html/HTMLMenuItemElement.h +++ b/dom/html/HTMLMenuItemElement.h @@ -125,7 +125,9 @@ protected: protected: virtual nsresult AfterSetAttr(int32_t aNamespaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; void WalkRadioGroup(Visitor* aVisitor); diff --git a/dom/html/HTMLMetaElement.cpp b/dom/html/HTMLMetaElement.cpp index 47effa2bc5..833c3177a7 100644 --- a/dom/html/HTMLMetaElement.cpp +++ b/dom/html/HTMLMetaElement.cpp @@ -60,7 +60,8 @@ HTMLMetaElement::SetMetaReferrer(nsIDocument* aDocument) nsresult HTMLMetaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None) { nsIDocument *document = GetUncomposedDoc(); @@ -82,7 +83,7 @@ HTMLMetaElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, } return nsGenericHTMLElement::AfterSetAttr(aNameSpaceID, aName, aValue, - aNotify); + aOldValue, aNotify); } nsresult diff --git a/dom/html/HTMLMetaElement.h b/dom/html/HTMLMetaElement.h index d3f05826d4..0b9117e557 100644 --- a/dom/html/HTMLMetaElement.h +++ b/dom/html/HTMLMetaElement.h @@ -33,7 +33,9 @@ public: bool aNullParent = true) override; virtual nsresult AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) override; + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, + bool aNotify) override; void CreateAndDispatchEvent(nsIDocument* aDoc, const nsAString& aEventName); diff --git a/dom/html/HTMLOptGroupElement.cpp b/dom/html/HTMLOptGroupElement.cpp index 73f51acf97..7467c5da38 100644 --- a/dom/html/HTMLOptGroupElement.cpp +++ b/dom/html/HTMLOptGroupElement.cpp @@ -102,7 +102,8 @@ HTMLOptGroupElement::RemoveChildAt(uint32_t aIndex, bool aNotify) nsresult HTMLOptGroupElement::AfterSetAttr(int32_t aNameSpaceID, nsIAtom* aName, - const nsAttrValue* aValue, bool aNotify) + const nsAttrValue* aValue, + const nsAttrValue* aOldValue, bool aNotify) { if (aNameSpaceID == kNameSpaceID_None && aName == nsGkAtoms::disabled) { // All our children