From 0a7e2d0150e0d65505580c3b8bb216e070b4de6d Mon Sep 17 00:00:00 2001 From: Gaming4JC Date: Sat, 26 Sep 2020 23:37:14 -0400 Subject: [PATCH 1/6] Issue #1656 - Fix broken comment from Part 1 Removing the vim line unintentionally broke the comment leading to build failure, this restores the comment. --- ipc/testshell/XPCShellEnvironment.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipc/testshell/XPCShellEnvironment.cpp b/ipc/testshell/XPCShellEnvironment.cpp index 68b72635f1..c97be68a52 100644 --- a/ipc/testshell/XPCShellEnvironment.cpp +++ b/ipc/testshell/XPCShellEnvironment.cpp @@ -1,5 +1,5 @@ /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ - * This Source Code Form is subject to the terms of the Mozilla Public +/* This Source Code Form is subject to the terms of the Mozilla Public * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ From 7f8852baa32cc1c26da6b47ebf59c0bd3191c090 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Tue, 29 Sep 2020 15:03:13 -0400 Subject: [PATCH 2/6] Issue #1643 - Follow up: Add a null check for mOwner in ResizeObserverNotificationHelper::Unregister A race condition seemed to exist between tab destruction and un-registering a ResizeObserver resulting in a null deref crash. The original reporter in Forum Topic 25311 experienced this on msn.com so that was the functional test reference. --- dom/base/ResizeObserverController.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dom/base/ResizeObserverController.cpp b/dom/base/ResizeObserverController.cpp index 7a6e6ba449..117e67fbfd 100644 --- a/dom/base/ResizeObserverController.cpp +++ b/dom/base/ResizeObserverController.cpp @@ -58,6 +58,12 @@ ResizeObserverNotificationHelper::Register() void ResizeObserverNotificationHelper::Unregister() { + if (!mOwner) { + // We've outlived our owner, so there's nothing registered anymore. + mRegistered = false; + return; + } + if (!mRegistered) { return; } From 2fae136204bc76de2a8a3c7f43a76a5e5bcdc598 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 30 Sep 2020 16:02:17 +0000 Subject: [PATCH 3/6] Issue #1643 - Follow-up: Make sure things aren't changed while iterating. This fixes some crashing scenarios. --- dom/base/ResizeObserverController.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/dom/base/ResizeObserverController.cpp b/dom/base/ResizeObserverController.cpp index 117e67fbfd..acc401a5e2 100644 --- a/dom/base/ResizeObserverController.cpp +++ b/dom/base/ResizeObserverController.cpp @@ -118,6 +118,10 @@ ResizeObserverController::Notify() return; } + // Hold a strong reference to the document, because otherwise calling + // all active observers on it might yank it out from under us. + RefPtr document(mDocument); + uint32_t shallowestTargetDepth = 0; GatherAllActiveObservations(shallowestTargetDepth); @@ -152,7 +156,7 @@ ResizeObserverController::Notify() nsEventStatus status = nsEventStatus_eIgnore; nsCOMPtr window = - mDocument->GetWindow()->GetCurrentInnerWindow(); + document->GetWindow()->GetCurrentInnerWindow(); if (window) { nsCOMPtr sgo = do_QueryInterface(window); @@ -184,7 +188,11 @@ ResizeObserverController::BroadcastAllActiveObservations() { uint32_t shallowestTargetDepth = UINT32_MAX; - for (auto observer : mResizeObservers) { + // Use a copy of the observers as this invokes the callbacks of the observers + // which could register/unregister observers at will. + nsTArray> tempObservers(mResizeObservers); + + for (auto observer : tempObservers) { uint32_t targetDepth = observer->BroadcastActiveObservations(); From 9f00ec1dfe50d8f841ef483a289c2070abfcaa54 Mon Sep 17 00:00:00 2001 From: athenian200 Date: Wed, 30 Sep 2020 14:05:56 -0500 Subject: [PATCH 4/6] Issue #1647 - Followup: Remove excessive VARIANT_OPACITY statements. I got very anxious about making sure I included VARIANT_OPACITY in all the places VARIANT_NUMBER was included to make sure it couldn't possibly break unexpectedly, and that led to me accidentally breaking a mechanism that prevented percentages from serializing as numbers in other parts of the code. It was a total accident, and these additions were unnecessary. Basically, the situation is that there was one part of the code where it determines what's allowed for the flex statement (and possibly other statements) by checking whether it got stored as a "number", and basically only disallows percentages if it attempted to store/serialize them as percentages. However, it only got to that part of the code because I accidentally allowed VARIANT_OPACITY as a valid way for certain tokens to parse where it wasn't necessary. If it tries to parse it that way under very specific circumstances... percentages will be marked valid and fed through the system as numbers rather than being rejected and not serialized at all, because the check to disallow percentages there relied on them being stored as percentages. It's a really weird thing to have a problem with in a lot of ways, because if percentages aren't allowed in a field, you would think people wouldn't try to use them there, much less depend on the broken behavior that results from them not parsing as a related value. --- layout/inspector/inDOMUtils.cpp | 2 +- layout/style/nsCSSParser.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/layout/inspector/inDOMUtils.cpp b/layout/inspector/inDOMUtils.cpp index a28b35ef8f..800201ce21 100644 --- a/layout/inspector/inDOMUtils.cpp +++ b/layout/inspector/inDOMUtils.cpp @@ -842,7 +842,7 @@ PropertySupportsVariant(nsCSSPropertyID aPropertyID, uint32_t aVariant) case eCSSProperty_grid_row_end: case eCSSProperty_font_weight: case eCSSProperty_initial_letter: - supported = VARIANT_NUMBER | VARIANT_OPACITY; + supported = VARIANT_NUMBER; break; default: diff --git a/layout/style/nsCSSParser.cpp b/layout/style/nsCSSParser.cpp index 09684a57ab..078cbaecdf 100644 --- a/layout/style/nsCSSParser.cpp +++ b/layout/style/nsCSSParser.cpp @@ -1299,7 +1299,7 @@ protected: } bool ParseNonNegativeNumber(nsCSSValue& aValue) { - return ParseSingleTokenNonNegativeVariant(aValue, VARIANT_NUMBER | VARIANT_OPACITY, nullptr); + return ParseSingleTokenNonNegativeVariant(aValue, VARIANT_NUMBER, nullptr); } // Helpers for some common ParseSingleTokenOneOrLargerVariant calls. @@ -1309,7 +1309,7 @@ protected: } bool ParseOneOrLargerNumber(nsCSSValue& aValue) { - return ParseSingleTokenOneOrLargerVariant(aValue, VARIANT_NUMBER | VARIANT_OPACITY, nullptr); + return ParseSingleTokenOneOrLargerVariant(aValue, VARIANT_NUMBER, nullptr); } // http://dev.w3.org/csswg/css-values/#custom-idents @@ -8485,7 +8485,7 @@ CSSParserImpl::ParseImageRect(nsCSSValue& aImage) break; } - static const int32_t VARIANT_SIDE = VARIANT_NUMBER | VARIANT_PERCENT | VARIANT_OPACITY; + static const int32_t VARIANT_SIDE = VARIANT_NUMBER | VARIANT_PERCENT; if (!ParseSingleTokenNonNegativeVariant(top, VARIANT_SIDE, nullptr) || !ExpectSymbol(',', true) || !ParseSingleTokenNonNegativeVariant(right, VARIANT_SIDE, nullptr) || @@ -10894,7 +10894,7 @@ CSSParserImpl::ParseWebkitGradientColorStop(nsCSSValueGradient* aGradient) if (mToken.mIdent.LowerCaseEqualsLiteral("color-stop")) { // Parse stop location, followed by comma. if (!ParseSingleTokenVariant(stop->mLocation, - VARIANT_NUMBER | VARIANT_PERCENT | VARIANT_OPACITY, + VARIANT_NUMBER | VARIANT_PERCENT, nullptr) || !ExpectSymbol(',', true)) { SkipUntil(')'); // Skip to end of color-stop(...) expression. @@ -16057,7 +16057,7 @@ static bool GetFunctionParseInformation(nsCSSKeyword aToken, {VARIANT_LBCALC, VARIANT_LBCALC, VARIANT_LBCALC}, {VARIANT_ANGLE_OR_ZERO}, {VARIANT_ANGLE_OR_ZERO, VARIANT_ANGLE_OR_ZERO}, - {VARIANT_NUMBER|VARIANT_OPACITY}, + {VARIANT_NUMBER}, {VARIANT_LENGTH|VARIANT_NONNEGATIVE_DIMENSION}, {VARIANT_LB|VARIANT_NONNEGATIVE_DIMENSION}, {VARIANT_NUMBER, VARIANT_NUMBER}, @@ -17639,7 +17639,7 @@ CSSParserImpl::ParseScrollSnapPoints(nsCSSValue& aValue, nsCSSPropertyID aPropID nsCSSKeywords::LookupKeyword(mToken.mIdent) == eCSSKeyword_repeat) { nsCSSValue lengthValue; if (ParseNonNegativeVariant(lengthValue, - VARIANT_LENGTH | VARIANT_PERCENT | VARIANT_OPACITY | VARIANT_CALC, + VARIANT_LENGTH | VARIANT_PERCENT | VARIANT_CALC, nullptr) != CSSParseResult::Ok) { REPORT_UNEXPECTED(PEExpectedNonnegativeNP); SkipUntil(')'); From 30bcf6f34736f5a44b1542833216c1993db80199 Mon Sep 17 00:00:00 2001 From: New Tobin Paradigm Date: Fri, 2 Oct 2020 05:13:43 -0400 Subject: [PATCH 5/6] Update CONTRIBUTING.md Revise Commit message style and add Follow-up form --- docs/CONTRIBUTING.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 9379ea45a6..031ddb9043 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -93,15 +93,20 @@ With rare exception, it is advisable to use the following style for commit messa This would directly apply to anyone making a pull request. +Single commits are where issues can be reasonably resolved in a single commit. - Single Commit: `Issue #xxx - Cited issue title or appropriate direct description of changes` - *`Issue #1083 - Deprecate FUEL extension helper javascript library`* -- Multi-Part Commits: `Issue #xxx - Part Number: Appropriate direct description of changes` + +Multi-part commits would be used for complex issues. However, an exception exists for multi-part commits where the issue is anticipated or ends up being long term such as "Stop using unified compilation of sources". In these instances the multi-part form is not required. +- Multi-Part Form: `Issue #xxx - Part Number: Appropriate direct description of changes` - *`Issue #492 - Part 1: Remove files`* - *`Issue #492 - Part 2: Build system, Installer/Packaging`* - *`Issue #492 - Part 3: nsUpdateService.js, updater.cpp, nsUpdateDriver.cpp`* - *`Issue #492 - Part 4: Remove superfluous brackets in nsUpdateService.js and updater.cpp`* -An exception exists for multi-part commits where the issue is anticipated or ends up being long term such as "Stop using unified compilation of sources". In these instances the multi-part form is not required. +Occasionally a resolved (and shipped) issue requires further changes to fix bugs. However, If follow-ups are complex enough to be multi-part it should be considered a new issue. +- Follow-up Form: *`Issue #xxx - Follow-up: Appropriate direct description of changes`* + - *`Issue #1643 - Follow-up: Make sure things aren't changed while iterating.`* ### Commits with no issue From 5465ff9914b77cbeeea51ecc13071a9f3facc9a5 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sun, 27 Sep 2020 16:48:30 +0000 Subject: [PATCH 6/6] [Pale-Moon] [downloads] Correct and simplify host handling. --- .../palemoon/components/downloads/DownloadsViewUI.jsm | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/application/palemoon/components/downloads/DownloadsViewUI.jsm b/application/palemoon/components/downloads/DownloadsViewUI.jsm index 7aa054d6a3..0220e566f3 100644 --- a/application/palemoon/components/downloads/DownloadsViewUI.jsm +++ b/application/palemoon/components/downloads/DownloadsViewUI.jsm @@ -80,20 +80,16 @@ this.DownloadsViewUI.DownloadElementShell.prototype = { get extendedDisplayName() { let s = DownloadsCommon.strings; - let [displayHost, fullHost] = DownloadUtils.getURIHost(this.download.source.url); + let displayHost = DownloadUtils.getURIHost(this.download.source.url); return s.statusSeparator(this.displayName, displayHost); }, get extendedDisplayNameTip() { let s = DownloadsCommon.strings; + let fullHost = DownloadUtils.getURIHost(this.download.source.url); let referrer = this.download.source.referrer; if (referrer) { - let [displayHost, fullHost] = DownloadUtils.getURIHost(this.download.source.url) + - ' (' + - DownloadUtils.getURIHost(referrer) + - ')'; - } else { - let [displayHost, fullHost] = DownloadUtils.getURIHost(this.download.source.url); + fullHost += ' (' + DownloadUtils.getURIHost(referrer) + ')'; } return s.statusSeparator(this.displayName, fullHost); },