Closed
Bug 657279
Opened 14 years ago
Closed 13 years ago
"ASSERTION: Didn't SaveReflowAndBoundingMetricsFor frame!"
Categories
(Core :: MathML, defect)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla15
People
(Reporter: jruderman, Assigned: fredw)
References
Details
(Keywords: assertion, testcase)
Attachments
(5 files, 3 obsolete files)
(deleted),
application/xhtml+xml
|
Details | |
(deleted),
text/plain
|
Details | |
(deleted),
text/html
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
karlt
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Comment 1•14 years ago
|
||
Comment 2•14 years ago
|
||
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•13 years ago
|
||
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•13 years ago
|
||
Assignee | ||
Comment 5•13 years ago
|
||
Assignee: nobody → fred.wang
Attachment #546413 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #604763 -
Flags: review?(karlt)
Assignee | ||
Comment 6•13 years ago
|
||
Attachment #604764 -
Flags: review?(karlt)
Assignee | ||
Comment 7•13 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 8•13 years ago
|
||
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 9•13 years ago
|
||
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 | ||
Comment 10•13 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
Comment 11•13 years ago
|
||
Yes, we can take these patches now.
The tests should help check that future refactoring leaves the code working as expected.
Assignee | ||
Comment 12•13 years ago
|
||
Assignee | ||
Comment 13•13 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•13 years ago
|
||
Assignee | ||
Comment 15•13 years ago
|
||
crashtests converted to reftests
Attachment #604763 -
Attachment is obsolete: true
Attachment #604764 -
Attachment is obsolete: true
Attachment #618696 -
Flags: review?(karlt)
Updated•13 years ago
|
Attachment #618696 -
Flags: review?(karlt) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 16•13 years ago
|
||
Comment 17•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/89cea655477a
http://hg.mozilla.org/mozilla-central/rev/a953fd717e46
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•