Closed
Bug 1063073
Opened 10 years ago
Closed 10 years ago
Make sure embedding elements that rely on an embedded SVG's intrinsic dimensions are resized if the SVG is late in loading
Categories
(Core :: SVG, defect)
Core
SVG
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: jwatt, Assigned: jwatt)
References
Details
Attachments
(1 file)
(deleted),
patch
|
dholbert
:
review+
lmandel
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This is to fix the bug described in bug 997101 comment 64.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8484432 -
Flags: review?(dholbert)
Comment 2•10 years ago
|
||
Comment on attachment 8484432 [details] [diff] [review]
patch
>diff --git a/layout/svg/nsSVGOuterSVGFrame.cpp b/layout/svg/nsSVGOuterSVGFrame.cpp
>+// helper
>+static inline bool
>+DependsOnIntrinsicSize(const nsIFrame* aEmbeddingFrame)
>+{
>+ const nsStylePosition *pos = aEmbeddingFrame->StylePosition();
>+ const nsStyleCoord &width = pos->mWidth;
>+ const nsStyleCoord &height = pos->mHeight;
>+
>+ // XXX it would be nice to know if the size of aEmbeddingFrame's containing
>+ // block depends on aEmbeddingFrame, then we'd know if we can return false
>+ // for eStyleUnit_Percent too.
>+ return !width.ConvertsToLength() ||
>+ !height.ConvertsToLength();
"!height" needs more indentation there. (It at least needs 2 spaces, or it can match the place you're moving it from, which had it aligned under "!width".)
> void
> nsSVGOuterSVGFrame::Init(nsIContent* aContent,
> nsContainerFrame* aParent,
> nsIFrame* aPrevInFlow)
> {
[...]
>+ nsIFrame* embeddingFrame;
>+ if (IsRootOfReplacedElementSubDoc(&embeddingFrame) && embeddingFrame) {
>+ if (DependsOnIntrinsicSize(embeddingFrame)) {
>+ // If this document loads after the embedding element has had its
>+ // reflow we need to schedule another reflow so that it will use our
>+ // intrinsic dimensions:
>+ embeddingFrame->PresContext()->PresShell()->
>+ FrameNeedsReflow(embeddingFrame, nsIPresShell::eStyleChange, NS_FRAME_IS_DIRTY);
>+ }
>+ }
IIUC, we don't need this if embeddingFrame is already marked as dirty, right?
So, maybe wrap this in:
if (NS_UNLIKELY(!embeddingFrame->HasAllStateBits(NS_FRAME_IS_DIRTY)) {
// Looks like this document is loading after the embedding element has had
// its first reflow, so we need to [...]
<make call to FrameNeedsReflow>
}
Attachment #8484432 -
Flags: review?(dholbert) → review+
Comment 3•10 years ago
|
||
Might this fix a bunch of existing intermittent-orange bugs? I seem to recall some related to SVG sizing.
Assignee | ||
Comment 4•10 years ago
|
||
There were a bunch that were filed as a result of the patch for bug 997101 landing, but those all went away when bug 997101 was backed out as a result. I don't think there are any existing orange bugs that this will fix.
Assignee | ||
Comment 5•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cc20221b0f6
As well as addressing the review comments I added a regression test before pushing.
Comment 6•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 7•10 years ago
|
||
Comment on attachment 8484432 [details] [diff] [review]
patch
Approval Request Comment
[Feature/regressing bug #]: NA
[User impact if declined]: NA
[Describe test coverage new/current, TBPL]: Landed on m-c for a while
[Risks and why]: Little, this patch only add one additional SVG reflow when the SVG is loaded in a different timing.
[String/UUID change made/needed]: NA
This patch is need to uplift bug 997101
Attachment #8484432 -
Flags: approval-mozilla-aurora?
Comment 8•10 years ago
|
||
Comment on attachment 8484432 [details] [diff] [review]
patch
Approving for Aurora so that we can take the perf fix for bug 997101.
Attachment #8484432 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•10 years ago
|
status-b2g-v2.1:
--- → affected
Updated•10 years ago
|
status-b2g-v2.2:
--- → fixed
Updated•10 years ago
|
Keywords: checkin-needed
Comment 9•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•