From 845188e8cdd30154099373dffa4c4644d85e598d Mon Sep 17 00:00:00 2001 From: Susanna Bowen Date: Fri, 15 Aug 2014 10:34:20 -0700 Subject: [PATCH] Bug 1030993 - Fix assertion failure in reftest css-ruby/ruby-whitespace-1.html. r=dbaron Fixes the assertion failure with text: "###!!! ASSERTION: Wrong line container hint: '!aForFrame || (aLineContainer == FindLineContainer(aForFrame) || aLineContainer->GetType() == nsGkAtoms::rubyTextContainerFrame || (aLineContainer->GetType() == nsGkAtoms::letterFrame && aLineContainer->IsFloating()))', file /home/sgbowen/builds/mozilla-central/layout/generic/nsTextFrame.cpp, line 1259" which occasionally appears when opening pages with ruby or when running ruby reftests. Updates the manifest for ruby reftests to the current expectations (adjust assertion counts, etc.) --- layout/base/nsCSSFrameConstructor.cpp | 6 ++++-- layout/generic/nsRubyBaseContainerFrame.cpp | 12 ++++++++++++ layout/generic/nsRubyBaseContainerFrame.h | 2 ++ layout/generic/nsRubyBaseFrame.cpp | 6 ++++++ layout/generic/nsRubyBaseFrame.h | 1 + layout/generic/nsRubyFrame.cpp | 6 ++++++ layout/generic/nsRubyFrame.h | 1 + layout/generic/nsRubyTextFrame.cpp | 10 ---------- layout/generic/nsTextFrame.cpp | 6 ++++-- layout/reftests/css-ruby/reftest.list | 2 +- 10 files changed, 37 insertions(+), 15 deletions(-) diff --git a/layout/base/nsCSSFrameConstructor.cpp b/layout/base/nsCSSFrameConstructor.cpp index 115feae12b5..bdc9a4f18b8 100644 --- a/layout/base/nsCSSFrameConstructor.cpp +++ b/layout/base/nsCSSFrameConstructor.cpp @@ -4577,7 +4577,8 @@ nsCSSFrameConstructor::FindDisplayData(const nsStyleDisplay* aDisplay, FCDATA_DESIRED_PARENT_TYPE_TO_BITS(eTypeRubyBaseContainer), NS_NewRubyBaseFrame) }, { NS_STYLE_DISPLAY_RUBY_BASE_CONTAINER, - FCDATA_DECL(FCDATA_DESIRED_PARENT_TYPE_TO_BITS(eTypeRuby), + FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT | + FCDATA_DESIRED_PARENT_TYPE_TO_BITS(eTypeRuby), NS_NewRubyBaseContainerFrame) }, { NS_STYLE_DISPLAY_RUBY_TEXT, FCDATA_DECL(FCDATA_IS_LINE_PARTICIPANT | @@ -9105,7 +9106,8 @@ nsCSSFrameConstructor::sPseudoParentData[eParentTypeCount] = { &nsCSSAnonBoxes::rubyBase }, { // Ruby Base Container - FCDATA_DECL(FCDATA_USE_CHILD_ITEMS | + FCDATA_DECL(FCDATA_USE_CHILD_ITEMS | + FCDATA_IS_LINE_PARTICIPANT | FCDATA_DESIRED_PARENT_TYPE_TO_BITS(eTypeRuby) | FCDATA_SKIP_FRAMESET, NS_NewRubyBaseContainerFrame), diff --git a/layout/generic/nsRubyBaseContainerFrame.cpp b/layout/generic/nsRubyBaseContainerFrame.cpp index b04c1184d50..814a92357f6 100644 --- a/layout/generic/nsRubyBaseContainerFrame.cpp +++ b/layout/generic/nsRubyBaseContainerFrame.cpp @@ -52,6 +52,12 @@ nsRubyBaseContainerFrame::GetFrameName(nsAString& aResult) const } #endif +/* virtual */ bool +nsRubyBaseContainerFrame::IsFrameOfType(uint32_t aFlags) const +{ + return nsContainerFrame::IsFrameOfType(aFlags & + ~(nsIFrame::eLineParticipant)); +} void nsRubyBaseContainerFrame::AppendTextContainer(nsIFrame* aFrame) { @@ -65,6 +71,12 @@ void nsRubyBaseContainerFrame::ClearTextContainers() { mTextContainers.Clear(); } +/* virtual */ bool +nsRubyBaseContainerFrame::CanContinueTextRun() const +{ + return true; +} + /* virtual */ void nsRubyBaseContainerFrame::Reflow(nsPresContext* aPresContext, nsHTMLReflowMetrics& aDesiredSize, diff --git a/layout/generic/nsRubyBaseContainerFrame.h b/layout/generic/nsRubyBaseContainerFrame.h index 85a161b99c8..4e939d7d88b 100644 --- a/layout/generic/nsRubyBaseContainerFrame.h +++ b/layout/generic/nsRubyBaseContainerFrame.h @@ -30,6 +30,8 @@ public: // nsIFrame overrides virtual nsIAtom* GetType() const MOZ_OVERRIDE; + virtual bool IsFrameOfType(uint32_t aFlags) const MOZ_OVERRIDE; + virtual bool CanContinueTextRun() const MOZ_OVERRIDE; virtual void Reflow(nsPresContext* aPresContext, nsHTMLReflowMetrics& aDesiredSize, const nsHTMLReflowState& aReflowState, diff --git a/layout/generic/nsRubyBaseFrame.cpp b/layout/generic/nsRubyBaseFrame.cpp index 447a7c349bd..005ccb0e29a 100644 --- a/layout/generic/nsRubyBaseFrame.cpp +++ b/layout/generic/nsRubyBaseFrame.cpp @@ -95,6 +95,12 @@ nsRubyBaseFrame::GetLogicalBaseline(WritingMode aWritingMode) const return mBaseline; } +/* virtual */ bool +nsRubyBaseFrame::CanContinueTextRun() const +{ + return true; +} + /* virtual */ void nsRubyBaseFrame::Reflow(nsPresContext* aPresContext, nsHTMLReflowMetrics& aDesiredSize, diff --git a/layout/generic/nsRubyBaseFrame.h b/layout/generic/nsRubyBaseFrame.h index 33012cf20eb..76a0ff38807 100644 --- a/layout/generic/nsRubyBaseFrame.h +++ b/layout/generic/nsRubyBaseFrame.h @@ -40,6 +40,7 @@ public: virtual bool IsFrameOfType(uint32_t aFlags) const MOZ_OVERRIDE; virtual nscoord GetLogicalBaseline(mozilla::WritingMode aWritingMode) const MOZ_OVERRIDE; + virtual bool CanContinueTextRun() const MOZ_OVERRIDE; #ifdef DEBUG_FRAME_DUMP virtual nsresult GetFrameName(nsAString& aResult) const MOZ_OVERRIDE; diff --git a/layout/generic/nsRubyFrame.cpp b/layout/generic/nsRubyFrame.cpp index 641a762b878..f6b3d7c4b97 100644 --- a/layout/generic/nsRubyFrame.cpp +++ b/layout/generic/nsRubyFrame.cpp @@ -164,6 +164,12 @@ nsRubyFrame::GetLogicalBaseline(WritingMode aWritingMode) const return mBaseline; } +/* virtual */ bool +nsRubyFrame::CanContinueTextRun() const +{ + return true; +} + /* virtual */ void nsRubyFrame::Reflow(nsPresContext* aPresContext, nsHTMLReflowMetrics& aDesiredSize, diff --git a/layout/generic/nsRubyFrame.h b/layout/generic/nsRubyFrame.h index b9c9096d6bf..bc88ce2ec42 100644 --- a/layout/generic/nsRubyFrame.h +++ b/layout/generic/nsRubyFrame.h @@ -38,6 +38,7 @@ public: nsReflowStatus& aStatus) MOZ_OVERRIDE; virtual nscoord GetLogicalBaseline(mozilla::WritingMode aWritingMode) const MOZ_OVERRIDE; + virtual bool CanContinueTextRun() const MOZ_OVERRIDE; #ifdef DEBUG_FRAME_DUMP virtual nsresult GetFrameName(nsAString& aResult) const MOZ_OVERRIDE; diff --git a/layout/generic/nsRubyTextFrame.cpp b/layout/generic/nsRubyTextFrame.cpp index 06c5da11e91..9ea2753c1db 100644 --- a/layout/generic/nsRubyTextFrame.cpp +++ b/layout/generic/nsRubyTextFrame.cpp @@ -75,9 +75,6 @@ nsRubyTextFrame::GetPrefISize(nsRenderingContext *aRenderingContext) nsRubyTextFrame::AddInlineMinISize(nsRenderingContext *aRenderingContext, nsIFrame::InlineMinISizeData *aData) { - // FIXME: See the fixme in AddInlinePrefISize. - aData->lineContainer = this; - for (nsFrameList::Enumerator e(PrincipalChildList()); !e.AtEnd(); e.Next()) { e.get()->AddInlineMinISize(aRenderingContext, aData); } @@ -87,13 +84,6 @@ nsRubyTextFrame::AddInlineMinISize(nsRenderingContext *aRenderingContext, nsRubyTextFrame::AddInlinePrefISize(nsRenderingContext *aRenderingContext, nsIFrame::InlinePrefISizeData *aData) { - // FIXME: We shouldn't need to set this, but it prevents us from tripping an - // assertion in nsTextFrame.cpp because FindLineContainer on a child frame will - // return the ruby text box (us) instead of the ruby text container (our - // parent). A fix would need to be made to FindLineContainer and/or - // CanContinueTextRun so that this line can be removed. - aData->lineContainer = this; - for (nsFrameList::Enumerator e(PrincipalChildList()); !e.AtEnd(); e.Next()) { e.get()->AddInlinePrefISize(aRenderingContext, aData); } diff --git a/layout/generic/nsTextFrame.cpp b/layout/generic/nsTextFrame.cpp index 9a17b119409..3e04c2ea3d1 100644 --- a/layout/generic/nsTextFrame.cpp +++ b/layout/generic/nsTextFrame.cpp @@ -1026,7 +1026,8 @@ private: static nsIFrame* FindLineContainer(nsIFrame* aFrame) { - while (aFrame && aFrame->CanContinueTextRun()) { + while (aFrame && (aFrame->IsFrameOfType(nsIFrame::eLineParticipant) || + aFrame->CanContinueTextRun())) { aFrame = aFrame->GetParent(); } return aFrame; @@ -1131,7 +1132,8 @@ CanTextCrossFrameBoundary(nsIFrame* aFrame, nsIAtom* aType) result.mScanSiblings = true; result.mTextRunCanCrossFrameBoundary = true; result.mLineBreakerCanCrossFrameBoundary = true; - } else if (aFrame->GetType() == nsGkAtoms::rubyTextFrame) { + } else if (aFrame->GetType() == nsGkAtoms::rubyTextFrame || + aFrame->GetType() == nsGkAtoms::rubyTextContainerFrame) { result.mFrameToScan = aFrame->GetFirstPrincipalChild(); result.mOverflowFrameToScan = aFrame->GetFirstChild(nsIFrame::kOverflowList); diff --git a/layout/reftests/css-ruby/reftest.list b/layout/reftests/css-ruby/reftest.list index 9a52cbf2599..e734d18a154 100644 --- a/layout/reftests/css-ruby/reftest.list +++ b/layout/reftests/css-ruby/reftest.list @@ -1,4 +1,4 @@ default-preferences pref(layout.css.ruby.enabled,true) -asserts(1) == ruby-whitespace-1.html ruby-whitespace-1-ref.html # bug 1052145 +fails asserts(3-7) == ruby-whitespace-1.html ruby-whitespace-1-ref.html # bug 1052924 == ruby-whitespace-2.html ruby-whitespace-2-ref.html