Closed Bug 1184842 Opened 9 years ago Closed 9 years ago

only consider the added and removed class names for restyling when a class="" attribute changes

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox42 --- fixed

People

(Reporter: heycam, Assigned: roc)

References

Details

Attachments

(12 files)

MozReview Request: Bug 1184842. Remove layout.frames.force_resizability pref. r=mats
40 bytes, text/x-review-board-request
MatsPalmgren_bugz
: review+
Details
MozReview Request: Bug 1184842. Remove frameset mVisibilityOverride since it's always false now. r=mats
40 bytes, text/x-review-board-request
MatsPalmgren_bugz
: review+
Details
MozReview Request: Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv
40 bytes, text/x-review-board-request
roc
: review+
Details
MozReview Request: Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz
40 bytes, text/x-review-board-request
roc
: review+
Details
MozReview Request: Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv
40 bytes, text/x-review-board-request
roc
: review+
Details
MozReview Request: Bug 1184842. Pass aOldValue to all mutation observers. r=peterv
40 bytes, text/x-review-board-request
peterv
: review+
Details
MozReview Request: Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv
40 bytes, text/x-review-board-request
peterv
: review+
Details
MozReview Request: Bug 1184842. Pass preparsed attribute values to nsNodeUtils::AttributeWillChange. r=peterv
40 bytes, text/x-review-board-request
peterv
: review+
Details
MozReview Request: Bug 1184842. Add aNewValue to nsIMutationObserver::AttributeWillChange. r=peterv
40 bytes, text/x-review-board-request
peterv
: review+
Details
MozReview Request: Bug 1184842. Preparse class attribute values in Element::BeforeSetAttr. r=peterv
40 bytes, text/x-review-board-request
peterv
: review+
Details
MozReview Request: Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam
40 bytes, text/x-review-board-request
roc
: review+
Details
MozReview Request: Bug 1184842. Restyling should consider only the classes that have changed. r=heycam
40 bytes, text/x-review-board-request
roc
: review+
Details 
Currently if we have content like:

  <style>
    .a .c { ... }
    .b > .d { ... }
  </style>
  <div class="a">
    <span class="d">
      <span>
        <span class="c"></span>
      </span>
    </span>
  </div>

and we do div.classList.toggle("b"), then we will restyle the div with eRestyle_Subtree.  This is because in nsCSSRuleProcessor::HasAttributeDependentStyle, we just go over the element's class list and look up all rules that use that class.  Because we call HasAttributeDependentStyle once for the AttributeWillChange notification and once for the AttributeChanged, we will first look at .a and find that we need an eRestyle_Subtree (due to the first rule in the sheet), and then in AttributeChanged we will look at .a and .b.

If we track which classes were added or removed from the attribute, we'd be able to look up only the .b and thus return an eRestyle_Self.
Summary: restyle more selectively when toggling a class on an element that has multiple classes → only consider the added and removed class names for restyling when a class="" attribute changes
(In reply to Cameron McCormack (:heycam) from comment #0)
> Currently if we have content like:
> 
>   <style>
>     .a .c { ... }
>     .b > .d { ... }
>   </style>

That should just be:

  .b { ... }

for the second rule, as I think we'll use eRestyle_Subtree even for the ">" case.
Assignee: nobody → roc
To make this work well we'll need to have the old and new values of 'class' available to one of the RestyleManager callbacks, either AttributeWillChange or AttributeChanged.

It seems that it would be generally for nsNodeUtils::AttributeChanged and all its callee callbacks to take the old attribute value as an nsAttrValue parameter. And it seems to me this can be done efficiently, by making nsAttrAndChildArray::SetAndTakeAttr always swap the old value into aValue --- i.e., just don't do the Reset() call it currently does --- so its callers can pass that in as the old value to nsNodeUtils::AttributeChanged. I think this would be basically free. Am I wrong?
Flags: needinfo?(peterv)
Seems fine by me. But I think bz knows more about our attributes implementation. Boris, does the approach in comment 2 sound ok to you?
Flags: needinfo?(peterv) → needinfo?(bzbarsky)
So here's the problem.  There are cases in which the new value of the attribute is not available during AttributeWillChange and the old value is not available during AttributeChanged.  The canonical example is the "style" attribute when foo.style.something gets set, but SVG attribute have some similar cases.  Basically, any time the nsAttrValue stores some object as its source of truth and that object exposes mutators directly...

Now maybe we can simply deoptimize in that case and explicitly not pass in the new value to AttributeWillChange or the old value to AttributeChanged; I'm a bit tired of this edge case tail wagging the attribute notification dog...  I agree that in an ideal world for class changes we'd just get a single notification with the old and new class lists or something.
Flags: needinfo?(bzbarsky)
Yeah, I discovered this problem while I was working offline this afternoon. I think I solved it just by making aOldValue a possibly-null parameter to AttributeChanged, but promising that it will be valid as long as either the old or new attribute value is not one of those problematic types (which I codify via a new nsAttrValue::StoresOwnData predicate method).
Turns out that it's not enough to pass the old value into AttributeChanged. We also need to pass the new value into AttributeWillChange, so that the style system can correctly compute which selectors currently apply to the element before the class is removed. In other words there still have to be two passes:
-- AttributeWillChange computes the classes that are being removed, and computes style changes for all rules that depend on those classes and are applying to the element before the change
-- AttributeDidChange computes the classes that are being added, and computes style changes for all rules that depend on those classes and are applying to the element after the change
Yes, that sounds right.

Also, yay tests.  ;)
Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv
Attachment #8637547 - Flags: review?(peterv)
Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz
Attachment #8637548 - Flags: review?(bzbarsky)
Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv
Attachment #8637550 - Flags: review?(peterv)
Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv

We will pass the preparsed value into AttributeWillChange.
Attachment #8637552 - Flags: review?(peterv)
Bug 1184842. Pass preparsed attribute values to nsNodeUtils::AttributeWillChange. r=peterv
Attachment #8637553 - Flags: review?(peterv)
Bug 1184842. Add aNewValue to nsIMutationObserver::AttributeWillChange. r=peterv
Attachment #8637554 - Flags: review?(peterv)
Bug 1184842. Preparse class attribute values in Element::BeforeSetAttr. r=peterv
Attachment #8637555 - Flags: review?(peterv)
Attachment #8637556 - Flags: review?(cam) → review+
Comment on attachment 8637556 [details]
MozReview Request: Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam

https://reviewboard.mozilla.org/r/13945/#review12469

Ship It!
Comment on attachment 8637557 [details]
MozReview Request: Bug 1184842. Restyling should consider only the classes that have changed. r=heycam

https://reviewboard.mozilla.org/r/13947/#review12467

::: layout/style/nsCSSRuleProcessor.cpp:2826
(Diff revision 1)
> +          nsTHashtable<nsPtrHashKey<nsIAtom> > otherClassesTable;

No need for the space before ">" these days.
Attachment #8637557 - Flags: review?(cam)
Comment on attachment 8637557 [details]
MozReview Request: Bug 1184842. Restyling should consider only the classes that have changed. r=heycam

https://reviewboard.mozilla.org/r/13947/#review12471

Ship It!
Attachment #8637557 - Flags: review+
> Remove layout.frames.force_resizability pref.

This was basically an accessibility pref, last I checked.  It's probably OK to kill it, but maybe check with the a11y folks?
Comment on attachment 8637548 [details]
MozReview Request: Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz

https://reviewboard.mozilla.org/r/13931/#review12555

::: dom/base/Element.h:480
(Diff revision 1)
> +  // aParsedValue receives the old value of the attribute.

This should mention under what conditions the old value that's output is useful.

::: dom/base/Element.h:1092
(Diff revision 1)
> +   *                      already useless. If the old value is StoresOwnData()

Really, people whose old value is !StoresOwnData() should be passing in a useful aOldValue.  The style system code does that....  I wonder whether we can assert that somehow.

Or is that the point the "it" that's labeled "already useless" is a bit ambiguous here.  Would be good to clarify.

::: dom/base/Element.h:1094
(Diff revision 1)
>     * @param aParsedValue  parsed new value of attribute. Replaced by the

We need to add documentation to the various nsAttrAndChildArray methods that do the value swapping that they do it.  Right now, nothing guarantees that they'll continue doing so, right?

::: dom/svg/nsSVGElement.h:350
(Diff revision 1)
> +  // aNewValue is replaced with the old value.

Again, document the caveats here.

::: dom/base/nsAttrValueInlines.h:201
(Diff revision 1)
> +  return t != eCSSStyleRule && !IsSVGType(t);

What about css::URLValue and css::ImageValue?  Looks to me like they could be shared across nsAttrValues.  I guess the important part is they're effectively immutable (at least in terms of their serialization)?  If so, please document.

r=me with the above dealt with.
Attachment #8637548 - Flags: review?(bzbarsky) → review+
Attachment #8637551 - Flags: review?(peterv)
Comment on attachment 8637551 [details]
MozReview Request: Bug 1184842. Pass aOldValue to all mutation observers. r=peterv

https://reviewboard.mozilla.org/r/13935/#review12659

::: dom/base/nsIMutationObserver.h:179
(Diff revision 1)
> -   * @param aNameSpaceID The namespace id of the changed attribute
> +   *g @param aNameSpaceID The namespace id of the changed attribute

Undo this.

::: dom/base/nsIMutationObserver.h:198
(Diff revision 1)
> +                                const nsAttrValue* aOldValue) = 0;

You need to change the IID for a change like this.
Comment on attachment 8637547 [details]
MozReview Request: Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv

https://reviewboard.mozilla.org/r/13929/#review12647
Attachment #8637547 - Flags: review?(peterv) → review+
Comment on attachment 8637550 [details]
MozReview Request: Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv

https://reviewboard.mozilla.org/r/13933/#review12657
Attachment #8637550 - Flags: review?(peterv) → review+
(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv

Does that mean that we'll now parse the value twice?
Flags: needinfo?(roc)
(In reply to Peter Van der Beken [:peterv] from comment #31)
> (In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #17)
> > Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv
> 
> Does that mean that we'll now parse the value twice?

No. If you look at the "prepare class attribute values" patch, we avoid parsing the class attribute again in ParseAttribute.
Flags: needinfo?(roc)
(In reply to Boris Zbarsky [:bz] from comment #26)
> This was basically an accessibility pref, last I checked.  It's probably OK
> to kill it, but maybe check with the a11y folks?

I checked; none of them had any reason to keep it.
https://reviewboard.mozilla.org/r/13931/#review12555

> This should mention under what conditions the old value that's output is useful.

Sure.

> Really, people whose old value is !StoresOwnData() should be passing in a useful aOldValue.  The style system code does that....  I wonder whether we can assert that somehow.
> 
> Or is that the point the "it" that's labeled "already useless" is a bit ambiguous here.  Would be good to clarify.

"it" refers to the value currently stored in the attribute. I'll clarify that.

SVG doesn't pass in a useful aOldValue for its magic attributes, except when it has already detected listeners, and I suspect that requiring it to do so would cause performance issues. So I don't want to impose such a requirement --- at least, not for this bug.

> We need to add documentation to the various nsAttrAndChildArray methods that do the value swapping that they do it.  Right now, nothing guarantees that they'll continue doing so, right?

There's only two, and they're both called SetAndSwapAttr, but sure, I'll add explicit comments about this.

> What about css::URLValue and css::ImageValue?  Looks to me like they could be shared across nsAttrValues.  I guess the important part is they're effectively immutable (at least in terms of their serialization)?  If so, please document.

True. Yes, since they're immutable, they shouldn't be a problem. I'll make this more clear.

> Again, document the caveats here.

OK.
https://reviewboard.mozilla.org/r/13935/#review12659

> Undo this.

OK

> You need to change the IID for a change like this.

Ah yes. Thanks!
Comment on attachment 8637545 [details]
MozReview Request: Bug 1184842. Remove layout.frames.force_resizability pref. r=mats

Bug 1184842. Remove layout.frames.force_resizability pref. r=mats
Comment on attachment 8637546 [details]
MozReview Request: Bug 1184842. Remove frameset mVisibilityOverride since it's always false now. r=mats

Bug 1184842. Remove frameset mVisibilityOverride since it's always false now. r=mats
Comment on attachment 8637547 [details]
MozReview Request: Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv

Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv
Attachment #8637547 - Flags: review+ → review?(peterv)
Attachment #8637548 - Flags: review+ → review?(bzbarsky)
Comment on attachment 8637548 [details]
MozReview Request: Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz

Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz
Comment on attachment 8637550 [details]
MozReview Request: Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv

Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv
Attachment #8637550 - Flags: review+ → review?(peterv)
Comment on attachment 8637551 [details]
MozReview Request: Bug 1184842. Pass aOldValue to all mutation observers. r=peterv

Bug 1184842. Pass aOldValue to all mutation observers. r=peterv
Attachment #8637551 - Flags: review?(peterv)
Comment on attachment 8637552 [details]
MozReview Request: Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv

Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv

We will pass the preparsed value into AttributeWillChange.
Comment on attachment 8637553 [details]
MozReview Request: Bug 1184842. Pass preparsed attribute values to nsNodeUtils::AttributeWillChange. r=peterv

Bug 1184842. Pass preparsed attribute values to nsNodeUtils::AttributeWillChange. r=peterv
Comment on attachment 8637554 [details]
MozReview Request: Bug 1184842. Add aNewValue to nsIMutationObserver::AttributeWillChange. r=peterv

Bug 1184842. Add aNewValue to nsIMutationObserver::AttributeWillChange. r=peterv
Comment on attachment 8637555 [details]
MozReview Request: Bug 1184842. Preparse class attribute values in Element::BeforeSetAttr. r=peterv

Bug 1184842. Preparse class attribute values in Element::BeforeSetAttr. r=peterv
Comment on attachment 8637556 [details]
MozReview Request: Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam

Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam
Attachment #8637556 - Flags: review+ → review?(cam)
Comment on attachment 8637557 [details]
MozReview Request: Bug 1184842. Restyling should consider only the classes that have changed. r=heycam

Bug 1184842. Restyling should consider only the classes that have changed. r=heycam
Attachment #8637557 - Flags: review+ → review?(cam)
Comment on attachment 8637547 [details]
MozReview Request: Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv

https://reviewboard.mozilla.org/r/13929/#review12741

Ship It!
Comment on attachment 8637548 [details]
MozReview Request: Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz

https://reviewboard.mozilla.org/r/13931/#review12743

Ship It!
Attachment #8637548 - Flags: review+
Comment on attachment 8637556 [details]
MozReview Request: Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam

https://reviewboard.mozilla.org/r/13945/#review12745

Ship It!
Attachment #8637556 - Flags: review+
Comment on attachment 8637557 [details]
MozReview Request: Bug 1184842. Restyling should consider only the classes that have changed. r=heycam

https://reviewboard.mozilla.org/r/13947/#review12747

Ship It!
Attachment #8637557 - Flags: review+
Comment on attachment 8637547 [details]
MozReview Request: Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv

carried forward r+
Attachment #8637547 - Flags: review?(peterv)
Comment on attachment 8637548 [details]
MozReview Request: Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz

carried forward r+
Attachment #8637548 - Flags: review?(bzbarsky)
Comment on attachment 8637556 [details]
MozReview Request: Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam

carried forward r+
Attachment #8637556 - Flags: review?(cam)
Comment on attachment 8637551 [details]
MozReview Request: Bug 1184842. Pass aOldValue to all mutation observers. r=peterv

https://reviewboard.mozilla.org/r/13935/#review12957
Attachment #8637551 - Flags: review?(peterv) → review+
Comment on attachment 8637552 [details]
MozReview Request: Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv

https://reviewboard.mozilla.org/r/13937/#review13045

::: dom/base/Element.cpp:2197
(Diff revision 2)
> +  }

Could you add a comment here that if there is a preparsed value we still call ParseAttribute because it might need to do stuff besides parsing (setting flags)?

::: dom/base/nsAttrValueOrString.h:58
(Diff revision 2)
> +  nsAttrValue* GetStoredAttrValue()

This needs documentation.

::: dom/svg/nsSVGElement.cpp:1442
(Diff revision 2)
> -  nsNodeUtils::AttributeWillChange(this, kNameSpaceID_None, aName, modType);
> +  nsNodeUtils::AttributeWillChange(this, kNameSpaceID_None, aName, modType,

This doesn't belong in this patch.
Attachment #8637552 - Flags: review?(peterv) → review+
Comment on attachment 8637555 [details]
MozReview Request: Bug 1184842. Preparse class attribute values in Element::BeforeSetAttr. r=peterv

https://reviewboard.mozilla.org/r/13943/#review13065

::: dom/base/Element.cpp:2377
(Diff revision 2)
> +      if (aValue && !aValue->GetAttrValue()) {

When is GetAttrValue non-null here? I'm asking because it seems like you're changing behaviour in that case. Before this patch we would always parse in ParseAttribute, but now we only parse if GetAttrValue returns null.
Attachment #8637555 - Flags: review?(peterv) → review+
Comment on attachment 8637553 [details]
MozReview Request: Bug 1184842. Pass preparsed attribute values to nsNodeUtils::AttributeWillChange. r=peterv

https://reviewboard.mozilla.org/r/13939/#review13067

::: dom/base/nsNodeUtils.h:52
(Diff revision 2)
> +   *                      preparsed it!!!

SetParsedAttr passes it unconditionally, right?
Attachment #8637553 - Flags: review?(peterv) → review+
Attachment #8637554 - Flags: review?(peterv) → review+
Comment on attachment 8637554 [details]
MozReview Request: Bug 1184842. Add aNewValue to nsIMutationObserver::AttributeWillChange. r=peterv

https://reviewboard.mozilla.org/r/13941/#review13069

::: dom/base/nsDOMMutationObserver.h:377
(Diff revision 2)
> -                        nsIDOMMutationEvent::MODIFICATION);
> +                        nsIDOMMutationEvent::MODIFICATION, nullptr);

So I guess this is ok because passing in a non-null value would just be a performance optimisation?
https://reviewboard.mozilla.org/r/13943/#review13065

> When is GetAttrValue non-null here? I'm asking because it seems like you're changing behaviour in that case. Before this patch we would always parse in ParseAttribute, but now we only parse if GetAttrValue returns null.

GetAttrValue is null when BeforeSetAttr is called by SetAttr, because in that case aValue is a string. GetAttrValue is non-null when BeforeSetAttr is called by SetParsedAttr, because in that case aValue is the parsed attribute value passed to SetParsedAttr. Now, SetParsedAttr is not called on "class", but I think this code makes sense if it was.

So I don't think this actually changes behavior. In practice we should always see null for aValue->GetAttrValue() here.
Comment on attachment 8637550 [details]
MozReview Request: Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv

Carrying forward r+ from comment 30.
Attachment #8637550 - Flags: review?(peterv) → review+
https://reviewboard.mozilla.org/r/13941/#review13069

> So I guess this is ok because passing in a non-null value would just be a performance optimisation?

Yes.
https://reviewboard.mozilla.org/r/13937/#review13045

> Could you add a comment here that if there is a preparsed value we still call ParseAttribute because it might need to do stuff besides parsing (setting flags)?

Sure.

> This needs documentation.

Right.

> This doesn't belong in this patch.

I'll shuffle the hunks around...
https://reviewboard.mozilla.org/r/13943/#review13065

> GetAttrValue is null when BeforeSetAttr is called by SetAttr, because in that case aValue is a string. GetAttrValue is non-null when BeforeSetAttr is called by SetParsedAttr, because in that case aValue is the parsed attribute value passed to SetParsedAttr. Now, SetParsedAttr is not called on "class", but I think this code makes sense if it was.
> 
> So I don't think this actually changes behavior. In practice we should always see null for aValue->GetAttrValue() here.

Ah, because SetParsedAttr doesn't call ParseAttribute. You're right.
Comment on attachment 8637545 [details]
MozReview Request: Bug 1184842. Remove layout.frames.force_resizability pref. r=mats

https://reviewboard.mozilla.org/r/13925/#review13271

I couldn't find any motivation in the bug comments for removing this feature but I'm guessing it's to remove the AttributeWillChange/AttributeChanged calls to avoid having to update them when the signatures change in later patches.

This pref was added in bug 105797 for the use case described in bug 89557 (to improve accessibility).  I think that's a valid use case.  Therefore I'm a bit reluctant to simply remove this pref without anything to replace it.

Adding support for the 'resize' property on <frame> might be a reasonable replacement (it's a bug we need to fix anyway).  The user can then add "frame { resize:both; overflow:auto; ... }" to userContent.css instead.

Would it be possible to synthesize dummy values for these two calls instead so that we can keep this feature until we have a replacement?
Attachment #8637545 - Flags: review?(mats)
see previous comment
Flags: needinfo?(roc)
There are some issues with this code:
-- Unlike all the other calls to AttributeWillChange/AttributeChanged, it does not call BeforeSetAttr or do the work that Element::SetAttrAndNotify does (calling AfterSetAttr, etc). So while we can pass dummy values here, the code does not and will not respect the invariants that apply for all the other calls.
-- I don't even know why we're calling these methods, since no attributes are changing.

I checked with our accessibility people, and none of them had ever heard of this pref.

This pref has never been exposed in UI AFAIK, and certainly isn't now, so anyone using it must have found it described on the Web and entered it using about:config. I think generally we don't promise support for such prefs.

Chrome doesn't have anything like this feature AFAICT.

We have bugs like https://bugzilla.mozilla.org/show_bug.cgi?id=57836 and https://bugzilla.mozilla.org/show_bug.cgi?id=219494 on frame resizing which had no comments at all for several years. I don't think anyone cares much about resizing framesets any more.

I think that keeping this code around is very much the wrong tradeoff.
Flags: needinfo?(roc)
OK, fair enough.
Attachment #8637545 - Flags: review+
Comment on attachment 8637545 [details]
MozReview Request: Bug 1184842. Remove layout.frames.force_resizability pref. r=mats

https://reviewboard.mozilla.org/r/13925/#review13359

Ship It!
Comment on attachment 8637546 [details]
MozReview Request: Bug 1184842. Remove frameset mVisibilityOverride since it's always false now. r=mats

https://reviewboard.mozilla.org/r/13927/#review13363

Ship It!
Attachment #8637546 - Flags: review?(mats) → review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/86d881ad9429355be61f271363b19d181fef366c
changeset:  86d881ad9429355be61f271363b19d181fef366c
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Tue Jul 21 16:18:17 2015 +1200
description:
Bug 1184842. Remove layout.frames.force_resizability pref. r=mats

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/4e87cabdf930af16ba0c1b79b258df220203b2e8
changeset:  4e87cabdf930af16ba0c1b79b258df220203b2e8
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Tue Jul 21 16:22:02 2015 +1200
description:
Bug 1184842. Remove frameset mVisibilityOverride since it's always false now. r=mats

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/1818b9f17a2648257aaa247f0620c6217e39483e
changeset:  1818b9f17a2648257aaa247f0620c6217e39483e
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Wed Jul 22 14:09:41 2015 +1200
description:
Bug 1184842. Change nsAttrAndChildArray::SetAndTakeAttr to nsAttrAndChildArray::SetAndSwapAttr. r=peterv

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/dd8314c68fb66569d9a44400e698b078e81b23c5
changeset:  dd8314c68fb66569d9a44400e698b078e81b23c5
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Sat Jul 25 17:57:13 2015 +1200
description:
Bug 1184842. Make SetAttrAndNotify use the real old value instead of aOldValue when possible. r=bz

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/3148cb83d613efb72814e5b0b10dc0f2bfbfbc7b
changeset:  3148cb83d613efb72814e5b0b10dc0f2bfbfbc7b
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Tue Jul 21 16:13:53 2015 +1200
description:
Bug 1184842. Add aOldValue parameter to nsNodeUtils::AttributeChanged. r=peterv

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/68a898398e3dbbb592b7c0b975e900cde95745c8
changeset:  68a898398e3dbbb592b7c0b975e900cde95745c8
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Sat Jul 25 18:01:19 2015 +1200
description:
Bug 1184842. Pass aOldValue to all mutation observers. r=peterv

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/727955f60cd89654b981ad1da8783e9b9f02b9f4
changeset:  727955f60cd89654b981ad1da8783e9b9f02b9f4
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Sat Aug 01 17:14:06 2015 +1200
description:
Bug 1184842. Allow BeforeSetAttr to preparse aValue. r=peterv

We will pass the preparsed value into AttributeWillChange.

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/728a2b8ad48db210e76df3f2bb8d1975173142d7
changeset:  728a2b8ad48db210e76df3f2bb8d1975173142d7
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Wed Jul 22 15:53:35 2015 +1200
description:
Bug 1184842. Pass preparsed attribute values to nsNodeUtils::AttributeWillChange. r=peterv

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/feb5a318bf0a60f7b9bf8cd1750268374deaf478
changeset:  feb5a318bf0a60f7b9bf8cd1750268374deaf478
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Sat Jul 25 18:05:19 2015 +1200
description:
Bug 1184842. Add aNewValue to nsIMutationObserver::AttributeWillChange. r=peterv

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/275bc8653fe3d43df382d33740cd1113f1eec4e0
changeset:  275bc8653fe3d43df382d33740cd1113f1eec4e0
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Sat Aug 01 17:46:15 2015 +1200
description:
Bug 1184842. Preparse class attribute values in Element::BeforeSetAttr. r=peterv

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/b2d75f57a96718c7df51006ee8096a6a5f1d32f9
changeset:  b2d75f57a96718c7df51006ee8096a6a5f1d32f9
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Wed Jul 22 15:54:07 2015 +1200
description:
Bug 1184842. Route aOldValue/aNewValue to AttributeData. r=heycam

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/ec4b62affc04cc329c12585fd00b0f3e17596124
changeset:  ec4b62affc04cc329c12585fd00b0f3e17596124
user:       Robert O'Callahan <robert@ocallahan.org>
date:       Sat Jul 25 17:56:58 2015 +1200
description:
Bug 1184842. Restyling should consider only the classes that have changed. r=heycam
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: