Bug 15232 - Allow persist to remove attributes. r=smaug

In a somewhat hacky way (by storing an special token). This should
probably be good enough for chrome code.

This is the root cause of bug 1792870 (before the regression we were
setting hidden="true/false" and thus storing the hidden attribute). Now
we're not because the attribute is removed, and since there are columns
that are hidden by default we'll never un-hide them.

Differential Revision: https://phabricator.services.mozilla.com/D158463
This commit is contained in:
Emilio Cobos Álvarez 2022-10-04 09:25:42 +00:00
parent 94acfaeaeb
commit 399f0690f9
3 changed files with 43 additions and 62 deletions

View File

@ -24,6 +24,11 @@ static bool IsRootElement(Element* aElement) {
return aElement->OwnerDoc()->GetRootElement() == aElement;
}
// FIXME: This is a hack to differentiate "attribute is missing" from "attribute
// is present but empty". Use a newline to avoid virtually all collisions.
// Ideally the XUL store would be able to store this more reasonably.
constexpr auto kMissingAttributeToken = u"-moz-missing\n"_ns;
static bool ShouldPersistAttribute(Element* aElement, nsAtom* aAttribute) {
if (IsRootElement(aElement)) {
// This is not an element of the top document, its owner is
@ -64,26 +69,26 @@ void XULPersist::AttributeChanged(dom::Element* aElement, int32_t aNameSpaceID,
const nsAttrValue* aOldValue) {
NS_ASSERTION(aElement->OwnerDoc() == mDocument, "unexpected doc");
if (aNameSpaceID != kNameSpaceID_None) {
return;
}
// See if there is anything we need to persist in the localstore.
//
// XXX Namespace handling broken :-(
nsAutoString persist;
// Persistence of attributes of xul:window is handled in AppWindow.
if (aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::persist, persist) &&
ShouldPersistAttribute(aElement, aAttribute) && !persist.IsEmpty() &&
if (aElement->GetAttr(nsGkAtoms::persist, persist) &&
ShouldPersistAttribute(aElement, aAttribute) &&
// XXXldb This should check that it's a token, not just a substring.
persist.Find(nsDependentAtomString(aAttribute)) >= 0) {
// Might not need this, but be safe for now.
nsCOMPtr<nsIDocumentObserver> kungFuDeathGrip(this);
nsContentUtils::AddScriptRunner(
NewRunnableMethod<Element*, int32_t, nsAtom*>(
"dom::XULPersist::Persist", this, &XULPersist::Persist, aElement,
kNameSpaceID_None, aAttribute));
nsContentUtils::AddScriptRunner(NewRunnableMethod<Element*, nsAtom*>(
"dom::XULPersist::Persist", this, &XULPersist::Persist, aElement,
aAttribute));
}
}
void XULPersist::Persist(Element* aElement, int32_t aNameSpaceID,
nsAtom* aAttribute) {
void XULPersist::Persist(Element* aElement, nsAtom* aAttribute) {
if (!mDocument) {
return;
}
@ -103,39 +108,14 @@ void XULPersist::Persist(Element* aElement, int32_t aNameSpaceID,
nsAutoString id;
aElement->GetAttr(kNameSpaceID_None, nsGkAtoms::id, id);
aElement->GetAttr(nsGkAtoms::id, id);
nsAtomString attrstr(aAttribute);
nsAutoString valuestr;
aElement->GetAttr(kNameSpaceID_None, aAttribute, valuestr);
nsAutoCString utf8uri;
nsresult rv = mDocument->GetDocumentURI()->GetSpec(utf8uri);
if (NS_WARN_IF(NS_FAILED(rv))) {
return;
}
NS_ConvertUTF8toUTF16 uri(utf8uri);
bool hasAttr;
#ifdef MOZ_NEW_XULSTORE
rv = XULStore::HasValue(uri, id, attrstr, hasAttr);
#else
rv = mLocalStore->HasValue(uri, id, attrstr, &hasAttr);
#endif
if (NS_WARN_IF(NS_FAILED(rv))) {
return;
}
if (hasAttr && valuestr.IsEmpty()) {
#ifdef MOZ_NEW_XULSTORE
rv = XULStore::RemoveValue(uri, id, attrstr);
NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "value removed");
#else
mLocalStore->RemoveValue(uri, id, attrstr);
#endif
return;
}
// Persisting attributes to top level windows is handled by AppWindow.
if (IsRootElement(aElement)) {
@ -145,6 +125,12 @@ void XULPersist::Persist(Element* aElement, int32_t aNameSpaceID,
}
}
NS_ConvertUTF8toUTF16 uri(utf8uri);
nsAutoString valuestr;
if (!aElement->GetAttr(aAttribute, valuestr)) {
valuestr = kMissingAttributeToken;
}
#ifdef MOZ_NEW_XULSTORE
rv = XULStore::SetValue(uri, id, attrstr, valuestr);
#else
@ -173,12 +159,6 @@ nsresult XULPersist::ApplyPersistentAttributes() {
}
#endif
ApplyPersistentAttributesInternal();
return NS_OK;
}
nsresult XULPersist::ApplyPersistentAttributesInternal() {
nsCOMArray<Element> elements;
nsAutoCString utf8uri;
@ -231,7 +211,7 @@ nsresult XULPersist::ApplyPersistentAttributesInternal() {
elements.AppendObject(element);
}
rv = ApplyPersistentAttributesToElements(id, elements);
rv = ApplyPersistentAttributesToElements(id, uri, elements);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
@ -241,21 +221,16 @@ nsresult XULPersist::ApplyPersistentAttributesInternal() {
}
nsresult XULPersist::ApplyPersistentAttributesToElements(
const nsAString& aID, nsCOMArray<Element>& aElements) {
nsAutoCString utf8uri;
nsresult rv = mDocument->GetDocumentURI()->GetSpec(utf8uri);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}
NS_ConvertUTF8toUTF16 uri(utf8uri);
const nsAString& aID, const nsAString& aDocURI,
nsCOMArray<Element>& aElements) {
nsresult rv = NS_OK;
// Get a list of attributes for which persisted values are available
#ifdef MOZ_NEW_XULSTORE
UniquePtr<XULStoreIterator> attrs;
rv = XULStore::GetAttrs(uri, aID, attrs);
rv = XULStore::GetAttrs(aDocURI, aID, attrs);
#else
nsCOMPtr<nsIStringEnumerator> attrs;
rv = mLocalStore->GetAttributeEnumerator(uri, aID, getter_AddRefs(attrs));
rv = mLocalStore->GetAttributeEnumerator(aDocURI, aID, getter_AddRefs(attrs));
#endif
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
@ -270,7 +245,7 @@ nsresult XULPersist::ApplyPersistentAttributesToElements(
}
nsAutoString value;
rv = XULStore::GetValue(uri, aID, attrstr, value);
rv = XULStore::GetValue(aDocURI, aID, attrstr, value);
#else
while (1) {
bool hasmore = PR_FALSE;
@ -310,7 +285,11 @@ nsresult XULPersist::ApplyPersistentAttributesToElements(
}
}
Unused << element->SetAttr(kNameSpaceID_None, attr, value, true);
if (value == kMissingAttributeToken) {
Unused << element->UnsetAttr(kNameSpaceID_None, attr, true);
} else {
Unused << element->SetAttr(kNameSpaceID_None, attr, value, true);
}
}
}

View File

@ -29,14 +29,13 @@ class XULPersist final : public nsStubDocumentObserver {
NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
protected:
void Persist(mozilla::dom::Element* aElement, int32_t aNameSpaceID,
nsAtom* aAttribute);
void Persist(mozilla::dom::Element* aElement, nsAtom* aAttribute);
private:
~XULPersist();
nsresult ApplyPersistentAttributes();
nsresult ApplyPersistentAttributesInternal();
nsresult ApplyPersistentAttributesToElements(const nsAString& aID,
const nsAString& aDocURI,
nsCOMArray<Element>& aElements);
#ifndef MOZ_NEW_XULSTORE

View File

@ -9,6 +9,7 @@
<button id="button1" label="Button1" persist="value"/>
<button id="button2" label="Button2" value="Normal" persist="value"/>
<button id="button3" label="Button3" value="Normal" persist="hidden" hidden="true"/>
<script>
<![CDATA[
@ -26,6 +27,7 @@ function runTest()
var firstRun = window.arguments[0];
var button1 = document.getElementById("button1");
var button2 = document.getElementById("button2");
var button3 = document.getElementById("button3");
if (firstRun) {
button1.setAttribute("value", "Pressed");
button2.removeAttribute("value");
@ -37,19 +39,20 @@ function runTest()
XULStore.persist(button2, "foo");
is(XULStore.hasValue(URI, "button2", "foo"), false, "attribute removed");
button3.removeAttribute("hidden");
window.close();
window.arguments[1].windowOpened();
}
else {
is(button1.getAttribute("value"), "Pressed",
"Attribute set");
is(button2.hasAttribute("value"), true,
"Attribute cleared");
is(button2.getAttribute("value"), "",
is(button2.hasAttribute("value"), false,
"Attribute cleared");
is(button2.hasAttribute("foo"), false,
"Attribute cleared");
is(button2.getAttribute("foo"), "",
is(button3.hasAttribute("hidden"), false,
"Attribute cleared");
window.close();