User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_9_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/33.0.1750.152 Safari/537.36 Steps to reproduce: Install FF26 or later on Mac or Windows (FF25 does not show this issue). Go to http://www.colorizephoto.com/ and move your mouse on to the main photo and then move the mouse so the circle following the mouse goes into the gray area around the photo. Actual results: The image suddenly jumps to the right when you move into the gray area. Expected results: The photo should not jump to the right. Note: the Inspector shows the correct right margins on the image in the center, but the horizontal scrollbar does not allow you to scroll to those margins. Vertical margins work correctly.

Regression window(m-i) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/9c8ab7e9ae41 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20131017025216 Bad: http://hg.mozilla.org/integration/mozilla-inbound/rev/45d9e6cd3473 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0 ID:20131017030414 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9c8ab7e9ae41&tochange=45d9e6cd3473 Regressed by: 45d9e6cd3473 Robert O'Callahan — Bug 926706. When nsGfxScrollFrameInner::UpdateOverflow decides we need to update the scrollbars, don't reflow the scrollframe with NS_FRAME_IS_DIRTY since that reflows all descendants. Just reflow the frame itself and don't dirty anything else. r=tn

Simplified repro demonstrating the issue. Clicking on the page will reposition a div and cause the scroll region to shrink.

Comment on attachment 8399411 [details] [diff] [review] fix > for (nsIFrame* lineFrame = line->mFirstChild; > n > 0; lineFrame = lineFrame->GetNextSibling(), --n) { > ConsiderChildOverflow(lineAreas, lineFrame); >+ if (line->IsInline()) { >+ // The overflow area of inline lines includes the margin-box of the >+ // frames in the line >+ lineAreas.UnionAllWith(lineFrame->GetMarginRectRelativeToSelf() + >+ lineFrame->GetPosition()); >+ } > } I don't see any code corresponding to this in the normal codepath (nsLineLayout::RelativePositionFrames). If I'd have found it, I'd just have asked you to put comments between it and this code linking the two together -- except I can't, which makes it seem like this is introducing a new inconsistency between the UpdateOverflow codepath and the reflow codepath rather than fixing one. I certainly could be missing something, though. >- return nsBlockFrameSuper::UpdateOverflow(); >+ nsOverflowAreas overflowAreas; >+ bool found; >+ nscoord bottomEdgeOfChildren = reinterpret_cast<nscoord> >+ (Properties().Get(BottomEdgeOfChildrenProperty(), &found)); >+ if (!found) { >+ bottomEdgeOfChildren = GetRect().height - GetUsedBorderAndPadding().bottom; >+ } >+ >+ ComputeOverflowAreas(nsRect(nsPoint(0, 0), GetSize()), StyleDisplay(), >+ bottomEdgeOfChildren, overflowAreas); >+ >+ return FinishAndStoreOverflow(overflowAreas, GetSize()); This change omits the view-resizing code in nsFrame::UpdateOverflow which seems (I think!) still important. Otherwise this seems fine, but marking review- because of those 2 issues.

Views are created for frames in the following cases: nsHTMLScrollFrame (nsCSSFrameConstructor::InitializeSelectFrame) nsSubdocumentFrame (nsSubdocumentFrame::Init) nsListControlFrame (nsCSSFrameConstructor::ConstructFrameFromItemInternal) nsObjectFrame (nsCSSFrameConstructor::ConstructFrameFromItemInternal) nsViewportFrame (nsDocumentViewer::MakeWindow, PresShell::VerifyIncrementalReflow, nsPrintEngine::SetRootView) nsMenuPopupFrame (nsMenuPopupFrame::CreatePopupView) So they're never created for block frames anymore. I'll add some assertions documenting this.

I removed the generic margin handling code that you pointed out was wrong, and fixed the bug properly by initializing the line overflow areas with the line bounds. I also fixed another bug which is that the previous patch ignored overflow contributions from some of the child lists (e.g. abs-pos). This caused failures on try.

Comment on attachment 8399837 [details] [diff] [review] Part 1: Assert that nsViews are only associated with a specific, limited set of frame types r=dbaron, although I prefer that assertion texts assert the thing you're asserting (which I think makes sense both reading the code and when the assertion fails) rather than having a message that only makes sense when the assertion fails. Maybe "only specific frame types can have an nsView"?

Comment on attachment 8399841 [details] [diff] [review] Part 3: Save a block's 'bottom edge of content' in a frame property if it doesn't match the block's content height, and use it in UpdateOverflow to compute the correct overflow >- nsOverflowAreas lineAreas; >+ nsRect bounds = line->mBounds; >+ nsOverflowAreas lineAreas(bounds, bounds); I hope this is equivalent to the nullptr == psd->mFrame case in nsLineLayout::RelativePositionFrames, which I find somewhat opaque, since it's using a bunch of stuff on the PerSpanData for the root span that I'm not in the mood to trace down the origins of. >+ // Union with child frames, skipping the principal and float lists >+ // since we already handled those using the line boxes. >+ nsLayoutUtils::UnionChildOverflow(this, overflowAreas, >+ kPrincipalList | kFloatList); Given that kSelectPopupList is used on a frame that derives from block, I think it would be good to include the default skip child lists as well. Maybe even make an enum constant on nsLayoutUtils for them? Or maybe just change patch 2 so the parameter is *additional* skip child lists, and it |s in the defaults? r=dbaron with that

https://hg.mozilla.org/integration/mozilla-inbound/rev/7c7f680de70e https://hg.mozilla.org/integration/mozilla-inbound/rev/82f376050e58 https://hg.mozilla.org/integration/mozilla-inbound/rev/c6becd0b9a5b

https://hg.mozilla.org/mozilla-central/rev/7c7f680de70e https://hg.mozilla.org/mozilla-central/rev/82f376050e58 https://hg.mozilla.org/mozilla-central/rev/c6becd0b9a5b