Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-27 14:52:16 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1917763 - More consistent handling of MathML's AttributeChanged(). r=emilio

Browse Source 
This patch tweaks implementation of AttributeChanged() in MathML layout
classes to be more consistent:

1. Only handle MathML-specific attributes in the default namespace.
2. Return immediately for class attributes, or forward the call to
   the parent class (or grandparent class).

Currently, nsMathMLContainerFrame::AttributeChanged() always forces a
reflow but this change does not cause new calls to it, except for the
edge case of accent/accentunder attributes in the non-default namespace
(nsMathMLmunderoverFrame). Not doing more for
nsMathMLmencloseFrame/nsMathMLmfracFrame breaks some tests, but we can
probably do a weaker invalidation. See bug 1918308 for a follow-up.

Differential Revision: https://phabricator.services.mozilla.com/D221921
This commit is contained in:
Frédéric Wang 2024-09-13 08:03:54 +00:00
parent d498e4b883
commit 5930f15160
7 changed files with 62 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
layout/mathml/nsMathMLContainerFrame.cpp
View File

@ -687,9 +687,10 @@ void nsMathMLContainerFrame::RemoveFrame(DestroyContext& aContext,
nsresult nsMathMLContainerFrame::AttributeChanged(int32_t aNameSpaceID,
nsAtom* aAttribute,
int32_t aModType) {
// XXX Since they are numerous MathML attributes that affect layout, and
// Since they are numerous MathML attributes that affect layout, and
// we can't check all of them here, play safe by requesting a reflow.
// XXXldb This should only do work for attributes that cause changes!
// TODO(bug 1918308): This should only do work for attributes that cause
// changes!
PresShell()->FrameNeedsReflow(
this, IntrinsicDirty::FrameAncestorsAndDescendants, NS_FRAME_IS_DIRTY);

7
layout/mathml/nsMathMLmencloseFrame.cpp
View File

@ -669,8 +669,13 @@ nscoord nsMathMLmencloseFrame::FixInterFrameSpacing(
nsresult nsMathMLmencloseFrame::AttributeChanged(int32_t aNameSpaceID,
nsAtom* aAttribute,
int32_t aModType) {
if (aAttribute == nsGkAtoms::notation_) {
if (aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::notation_) {
InitNotations();
// TODO(bug 1918308): This was copied from nsMathMLContainerFrame and seems
// necessary for some invalidation tests, but we can probably do less.
PresShell()->FrameNeedsReflow(
this, IntrinsicDirty::FrameAncestorsAndDescendants, NS_FRAME_IS_DIRTY);
return NS_OK;
}
return nsMathMLContainerFrame::AttributeChanged(aNameSpaceID, aAttribute,

8
layout/mathml/nsMathMLmfracFrame.cpp
View File

@ -120,8 +120,14 @@ void nsMathMLmfracFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
nsresult nsMathMLmfracFrame::AttributeChanged(int32_t aNameSpaceID,
nsAtom* aAttribute,
int32_t aModType) {
if (nsGkAtoms::linethickness_ == aAttribute) {
if (aNameSpaceID == kNameSpaceID_None &&
nsGkAtoms::linethickness_ == aAttribute) {
InvalidateFrame();
// TODO(bug 1918308): This was copied from nsMathMLContainerFrame and seems
// necessary for some invalidation tests, but we can probably do less.
PresShell()->FrameNeedsReflow(
this, IntrinsicDirty::FrameAncestorsAndDescendants, NS_FRAME_IS_DIRTY);
return NS_OK;
}
return nsMathMLContainerFrame::AttributeChanged(aNameSpaceID, aAttribute,
aModType);

3
layout/mathml/nsMathMLmsqrtFrame.cpp
View File

@ -47,6 +47,9 @@ nsMathMLmsqrtFrame::InheritAutomaticData(nsIFrame* aParent) {
nsresult nsMathMLmsqrtFrame::AttributeChanged(int32_t aNameSpaceID,
nsAtom* aAttribute,
int32_t aModType) {
// Skip nsMathMLmencloseFrame::AttributeChanged, since msqrt does not accept
// the notation attribute.
// TODO(bug 1918310): msqrt should share its logic with mroot instead.
return nsMathMLContainerFrame::AttributeChanged(aNameSpaceID, aAttribute,
aModType);
}

4
layout/mathml/nsMathMLmsqrtFrame.h
View File

@ -37,9 +37,7 @@ These attributes are inherited by every element from its rendering environment,
but can be set explicitly only on <mstyle>. (See Section 3.3.4.)
*/
// XXXfredw: This class should share its layout logic with nsMathMLmrootFrame
// when the menclose "radical" notation is removed.
// See https://bugzilla.mozilla.org/show_bug.cgi?id=1548522
// TODO(bug 1918310): msqrt should share its logic with mroot instead.
class nsMathMLmsqrtFrame final : public nsMathMLmencloseFrame {
public:
NS_DECL_FRAMEARENA_HELPERS(nsMathMLmsqrtFrame)

65
layout/mathml/nsMathMLmtableFrame.cpp
View File

@ -665,7 +665,7 @@ nsresult nsMathMLmtableWrapperFrame::AttributeChanged(int32_t aNameSpaceID,
if (!rgFrame || !rgFrame->IsTableRowGroupFrame()) return NS_OK;
// align - just need to issue a dirty (resize) reflow command
if (aAttribute == nsGkAtoms::align) {
if (aNameSpaceID == kNameSpaceID_None && aAttribute == nsGkAtoms::align) {
PresShell()->FrameNeedsReflow(this, IntrinsicDirty::None,
NS_FRAME_IS_DIRTY);
return NS_OK;
@ -674,7 +674,8 @@ nsresult nsMathMLmtableWrapperFrame::AttributeChanged(int32_t aNameSpaceID,
// displaystyle - may seem innocuous, but it is actually very harsh --
// like changing an unit. Blow away and recompute all our automatic
// presentational data, and issue a style-changed reflow request
if (aAttribute == nsGkAtoms::displaystyle_) {
if (aNameSpaceID == kNameSpaceID_None &&
aAttribute == nsGkAtoms::displaystyle_) {
nsMathMLContainerFrame::RebuildAutomaticDataForChildren(GetParent());
// Need to reflow the parent, not us, because this can actually
// affect siblings.
@ -686,32 +687,37 @@ nsresult nsMathMLmtableWrapperFrame::AttributeChanged(int32_t aNameSpaceID,
// ...and the other attributes affect rows or columns in one way or another
if (aAttribute == nsGkAtoms::rowspacing_ ||
aAttribute == nsGkAtoms::columnspacing_ ||
aAttribute == nsGkAtoms::framespacing_) {
if (aNameSpaceID == kNameSpaceID_None &&
(aAttribute == nsGkAtoms::rowspacing_ ||
aAttribute == nsGkAtoms::columnspacing_ ||
aAttribute == nsGkAtoms::framespacing_)) {
nsMathMLmtableFrame* mathMLmtableFrame = do_QueryFrame(tableFrame);
if (mathMLmtableFrame) {
ParseSpacingAttribute(mathMLmtableFrame, aAttribute);
mathMLmtableFrame->SetUseCSSSpacing();
}
} else if (aAttribute == nsGkAtoms::rowalign_ ||
aAttribute == nsGkAtoms::rowlines_ ||
aAttribute == nsGkAtoms::columnalign_ ||
aAttribute == nsGkAtoms::columnlines_) {
PresShell()->FrameNeedsReflow(
this, IntrinsicDirty::FrameAncestorsAndDescendants, NS_FRAME_IS_DIRTY);
return NS_OK;
}
if (aNameSpaceID == kNameSpaceID_None &&
(aAttribute == nsGkAtoms::rowalign_ ||
aAttribute == nsGkAtoms::rowlines_ ||
aAttribute == nsGkAtoms::columnalign_ ||
aAttribute == nsGkAtoms::columnlines_)) {
// clear any cached property list for this table
tableFrame->RemoveProperty(AttributeToProperty(aAttribute));
// Reparse the new attribute on the table.
ParseFrameAttribute(tableFrame, aAttribute, true);
} else {
// Ignore attributes that do not affect layout.
PresShell()->FrameNeedsReflow(
this, IntrinsicDirty::FrameAncestorsAndDescendants, NS_FRAME_IS_DIRTY);
return NS_OK;
}
// Explicitly request a reflow in our subtree to pick up any changes
PresShell()->FrameNeedsReflow(
this, IntrinsicDirty::FrameAncestorsAndDescendants, NS_FRAME_IS_DIRTY);
return NS_OK;
// Skip nsTableWrapperFrame::AttributeChanged, mtable does not share more
// attributes with table.
return nsContainerFrame::AttributeChanged(aNameSpaceID, aAttribute, aModType);
}
nsIFrame* nsMathMLmtableWrapperFrame::GetRowFrameAt(int32_t aRowIndex) {
@ -1024,9 +1030,13 @@ nsresult nsMathMLmtrFrame::AttributeChanged(int32_t aNameSpaceID,
// rowalign : Here
// columnalign : Here
if (aAttribute != nsGkAtoms::rowalign_ &&
aAttribute != nsGkAtoms::columnalign_) {
return NS_OK;
if (aNameSpaceID != kNameSpaceID_None ||
(aAttribute != nsGkAtoms::rowalign_ &&
aAttribute != nsGkAtoms::columnalign_)) {
// Skip nsTableCellFrame::AttributeChanged, mtr does not share any attribute
// with tr.
return nsContainerFrame::AttributeChanged(aNameSpaceID, aAttribute,
aModType);
}
RemoveProperty(AttributeToProperty(aAttribute));
@ -1075,8 +1085,9 @@ nsresult nsMathMLmtdFrame::AttributeChanged(int32_t aNameSpaceID,
// rowspan : here
// columnspan : here
if (aAttribute == nsGkAtoms::rowalign_ ||
aAttribute == nsGkAtoms::columnalign_) {
if (aNameSpaceID == kNameSpaceID_None &&
(aAttribute == nsGkAtoms::rowalign_ ||
aAttribute == nsGkAtoms::columnalign_)) {
RemoveProperty(AttributeToProperty(aAttribute));
// Reparse the attribute.
@ -1084,15 +1095,17 @@ nsresult nsMathMLmtdFrame::AttributeChanged(int32_t aNameSpaceID,
return NS_OK;
}
if (aAttribute == nsGkAtoms::rowspan ||
aAttribute == nsGkAtoms::columnspan_) {
// use the naming expected by the base class
if (aAttribute == nsGkAtoms::columnspan_) aAttribute = nsGkAtoms::colspan;
if (aNameSpaceID == kNameSpaceID_None &&
(aAttribute == nsGkAtoms::rowspan ||
aAttribute == nsGkAtoms::columnspan_)) {
// nsTableCellFrame takes care of renaming columnspan to colspan.
return nsTableCellFrame::AttributeChanged(aNameSpaceID, aAttribute,
aModType);
}
return NS_OK;
// Skip nsTableCellFrame::AttributeChanged, mtd does not share more attributes
// with td.
return nsContainerFrame::AttributeChanged(aNameSpaceID, aAttribute, aModType);
}
StyleVerticalAlignKeyword nsMathMLmtdFrame::GetVerticalAlign() const {

5
layout/mathml/nsMathMLmunderoverFrame.cpp
View File

@ -39,8 +39,9 @@ nsMathMLmunderoverFrame::~nsMathMLmunderoverFrame() = default;
nsresult nsMathMLmunderoverFrame::AttributeChanged(int32_t aNameSpaceID,
nsAtom* aAttribute,
int32_t aModType) {
if (nsGkAtoms::accent_ == aAttribute ||
nsGkAtoms::accentunder_ == aAttribute) {
if (aNameSpaceID == kNameSpaceID_None &&
(nsGkAtoms::accent_ == aAttribute ||
nsGkAtoms::accentunder_ == aAttribute)) {
// When we have automatic data to update within ourselves, we ask our
// parent to re-layout its children
return ReLayoutChildren(GetParent());