The default bug view has changed. See this FAQ.

"ASSERTION: Didn't SaveReflowAndBoundingMetricsFor frame!"

RESOLVED FIXED in mozilla15

Status

()

Core
MathML
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: Jesse Ruderman, Assigned: fredw)

Tracking

(Blocks: 2 bugs, {assertion, testcase})

Trunk
mozilla15
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Created attachment 532560 [details]
testcase
(Reporter)

Comment 1

6 years ago
Created attachment 532561 [details]
stack trace
Looks like a regression from bug 21479.

The whole embellished hierarchy may change if the selected subexpression changes from/to an embellished operator.
Blocks: 21479
(Assignee)

Comment 3

6 years ago
Created attachment 546412 [details]
testcase2

Testcase with various kinds of change.
There seems to be a separate bug when selected is changed on the <math/> element (m7 and m8).
(Assignee)

Comment 4

6 years ago
Created attachment 546413 [details] [diff] [review]
maction: transmit automatic data when the selected child changes
(Assignee)

Updated

5 years ago
Blocks: 544036
(Assignee)

Comment 5

5 years ago
Created attachment 604763 [details] [diff] [review]
546413: maction: transmit automatic data when the selected child changes - V2
Assignee: nobody → fred.wang
Attachment #546413 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #604763 - Flags: review?(karlt)
(Assignee)

Comment 6

5 years ago
Created attachment 604764 [details] [diff] [review]
crashtests
Attachment #604764 - Flags: review?(karlt)
(Assignee)

Comment 7

5 years ago
> There seems to be a separate bug when selected is changed on the <math/>
> element (m7 and m8).

I opened bug 734729
Comment on attachment 604764 [details] [diff] [review]
crashtests

Can this be turned into a reftest against a static dom to check that rendering is as expected?
Reftests also fail when there are assertions.

> load 557474-1.html
> load 655451-1.xhtml
> load 654928-1.html
>+load bug-657279.html

Looks like the filename convention here is a little different.
Attachment #604764 - Flags: review?(karlt) → review+
Comment on attachment 604763 [details] [diff] [review]
546413: maction: transmit automatic data when the selected child changes - V2

I assume this fixes the assertion for these testcases but I fear there will be other occurrences of the assertion.

There is a general problem that when a frame changes to or from an embellished
operator, the parent frame needs to recheck whether or not it is an
embellished operator.

Perhaps we need a method similar to TransmitAutomaticData that propagates up
the hierarchy until there is no change to the NS_MATHML_EMBELLISH_OPERATOR
flag (and perhaps to other info in mEmbellishData and to NS_MATHML_SPACE_LIKE).  However, changing the existing TransmitAutomaticData() to do this may be inefficent if it continues to be called from RebuildAutomaticDataForChildren(), which calls TransmitAutomaticData on all children and then on their parent.

I can't remember exactly what else is done in TransmitAutomaticData (in
various implementations), but perhaps we should rethink what
RebuildAutomaticDataForChildren() does, and instead handle changes that
propagate up the hierarchy (i.e. TransmitAutomaticData calls) separately.  I guess such changes would include the NS_MATHML_SPACE_LIKE flag and PresentationData::baseFrame.

However there are some other things to bear in mind:

  Attribute changes on mstyle (and math) need to trigger this
  TransmitAutomaticData() in descendant frames.  This is currently done through
  ReLayoutChildren().

  Check that calling TransmitAutomaticData during Reflow()
  (as the maction frame does) is early enough to propagate to
  TransmitAutomaticData() on the parent, given the parent has begun reflow at
  this stage.  This may be fine because Reflow of a frame typically depends
  on Reflow of the children.  But AttributeChanged is probably a more
  appropriate place than BuildDisplayList, Reflow, and Place to check the
  "selected" attribute.
Attachment #604763 - Flags: review?(karlt) → review+
(Assignee)

Updated

5 years ago
Blocks: 744783
(Assignee)

Comment 10

5 years ago
Karl, do you think we should take these patches now (with crashtests converted to reftests) and handle the refactoring of these data transmission in another bug? I expect Andrii to improve a lot our code for dynamic changes this summer and your suggestion of comment 9 looks something that can fit in his project.
OS: Mac OS X → All
Hardware: x86_64 → All
Yes, we can take these patches now.
The tests should help check that future refactoring leaves the code working as expected.
(Assignee)

Comment 12

5 years ago
https://tbpl.mozilla.org/?tree=Try&rev=4f9fd5558ccc
(Assignee)

Comment 13

5 years ago
(In reply to Frédéric Wang from comment #12)
> https://tbpl.mozilla.org/?tree=Try&rev=4f9fd5558ccc

I forgot to handle the reftest-wait class when I converted the crashtests into reftests.

https://tbpl.mozilla.org/?tree=Try&rev=1181eb8fc887
(Assignee)

Comment 14

5 years ago
Created attachment 618692 [details] [diff] [review]
546413: maction: transmit automatic data when the selected child changes - V3
(Assignee)

Comment 15

5 years ago
Created attachment 618696 [details] [diff] [review]
Reftests

crashtests converted to reftests
Attachment #604763 - Attachment is obsolete: true
Attachment #604764 - Attachment is obsolete: true
Attachment #618696 - Flags: review?(karlt)
Attachment #618696 - Flags: review?(karlt) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/89cea655477a
https://hg.mozilla.org/integration/mozilla-inbound/rev/a953fd717e46
Flags: in-testsuite+
Keywords: checkin-needed
Target Milestone: --- → mozilla15
http://hg.mozilla.org/mozilla-central/rev/89cea655477a
http://hg.mozilla.org/mozilla-central/rev/a953fd717e46
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.