Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(148)

Issue 349730043: WIP - wip

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 months, 3 weeks ago by vollick
Modified:
4 months, 2 weeks ago
Reviewers:
ajuma
Visibility:
Public.

Description

WIP - wip wip BUG=

Patch Set 1 #

Total comments: 20

Patch Set 2 : first attempt at scaling the root margin #

Patch Set 3 : . #

Patch Set 4 : . #

Patch Set 5 : . #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+671 lines, -34 lines) Patch
M LayoutTests/intersection-observer/intersection-observer-entry-interface.html View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/intersection-observer/intersection-observer-interface.html View 1 chunk +0 lines, -1 line 0 comments Download
M Source/WebCore/dom/Document.h View 1 2 4 chunks +22 lines, -0 lines 0 comments Download
M Source/WebCore/dom/Document.cpp View 1 2 3 4 5 chunks +303 lines, -1 line 2 comments Download
M Source/WebCore/dom/Element.h View 4 chunks +25 lines, -5 lines 0 comments Download
M Source/WebCore/dom/Element.cpp View 1 2 3 4 chunks +88 lines, -0 lines 0 comments Download
M Source/WebCore/dom/ElementRareData.h View 3 chunks +10 lines, -0 lines 0 comments Download
M Source/WebCore/dom/ElementRareData.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/WebCore/page/FrameView.cpp View 1 chunk +3 lines, -0 lines 0 comments Download
M Source/WebCore/page/IntersectionObserver.h View 2 chunks +33 lines, -10 lines 0 comments Download
M Source/WebCore/page/IntersectionObserver.cpp View 1 chunk +117 lines, -6 lines 0 comments Download
M Source/WebCore/page/IntersectionObserver.idl View 2 chunks +4 lines, -1 line 0 comments Download
M Source/WebCore/page/IntersectionObserverEntry.h View 1 2 3 chunks +8 lines, -0 lines 0 comments Download
M Source/WebCore/page/IntersectionObserverEntry.cpp View 2 chunks +31 lines, -0 lines 0 comments Download
M Source/WebCore/platform/Logging.h View 1 chunk +1 line, -0 lines 0 comments Download
M Source/WebCore/platform/graphics/FloatRect.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/WebCore/rendering/RenderBlock.h View 1 chunk +2 lines, -0 lines 0 comments Download
M Source/WebCore/rendering/RenderBlock.cpp View 1 chunk +9 lines, -0 lines 0 comments Download
M Source/WebCore/rendering/RenderObject.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/WebKit/Shared/WebPreferences.yaml View 2 chunks +9 lines, -8 lines 0 comments Download

Messages

Total messages: 3
ajuma
Some questions about the implicit root logic, and some tiny nits, but other than that ...
4 months, 3 weeks ago (2018-06-25 17:54:38 UTC) #1
vollick
https://codereview.appspot.com/349730043/diff/1/Source/WebCore/dom/Document.cpp File Source/WebCore/dom/Document.cpp (right): https://codereview.appspot.com/349730043/diff/1/Source/WebCore/dom/Document.cpp#newcode7375 Source/WebCore/dom/Document.cpp:7375: if (!root || (root->isConnected() && &root->document() == this)) On ...
4 months, 2 weeks ago (2018-07-03 15:22:29 UTC) #2
ajuma
4 months, 2 weeks ago (2018-07-03 18:21:48 UTC) #3
https://codereview.appspot.com/349730043/diff/1/Source/WebCore/dom/Document.cpp
File Source/WebCore/dom/Document.cpp (right):

https://codereview.appspot.com/349730043/diff/1/Source/WebCore/dom/Document.c...
Source/WebCore/dom/Document.cpp:7424: // FIXME: test with zoom.
On 2018/07/03 15:22:28, vollick wrote:
> On 2018/06/25 17:54:37, ajuma wrote:
> > Is testing with zoom worth doing now (perhaps leaving the work of fixing
> > behavior with zoom to later)?
> 
> I checked zoom, and it seemed wrong. I attempted to fix this, but I'm not
> confident I'm on the right track and would like to chat with you at some
point.

Sure, sgtm.

https://codereview.appspot.com/349730043/diff/1/Source/WebCore/dom/Element.cpp
File Source/WebCore/dom/Element.cpp (right):

https://codereview.appspot.com/349730043/diff/1/Source/WebCore/dom/Element.cp...
Source/WebCore/dom/Element.cpp:3247: // FIXME.
On 2018/07/03 15:22:29, vollick wrote:
> On 2018/06/25 17:54:38, ajuma wrote:
> > Is this something like scheduling layout on the root document?
> 
> I thought it was scheduling something that would guarantee that we'd call
> updateIntersectionObservations. I could add a corresponding scheduleFOO
function
> to Document and call it here, maybe?

That sounds like a reasonable approach.

https://codereview.appspot.com/349730043/diff/80001/Source/WebCore/dom/Docume...
File Source/WebCore/dom/Document.cpp (right):

https://codereview.appspot.com/349730043/diff/80001/Source/WebCore/dom/Docume...
Source/WebCore/dom/Document.cpp:7438: // Page zoom should not affect visiual
viewport, so we will only set the
Typo: visual

https://codereview.appspot.com/349730043/diff/80001/Source/WebCore/dom/Docume...
Source/WebCore/dom/Document.cpp:7439: // scale factor here, for the case where
the root element is specified.
See the "Coordinate systems" comment in FrameView.h (it's around line 426).
Maybe you could rephrase this comment and the math done in this function using
the terminology defined there?
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b