Open Bug 1553013 Opened 2 years ago Updated 27 days ago

Split PresShell::ScrollFrameRectIntoView into functions, one is for cross doc, the other is in process doc

Categories

(Core :: Layout, enhancement, P2)

enhancement

Tracking

()

ASSIGNED
Fission Milestone M8

People

(Reporter: hiro, Assigned: hiro)

References

(Blocks 1 open bug)

Details

Similar to bug 1553012, but this is for selection. For selection we need to use PresShell::ScrollFrameRectIntoView across document boundaries. layout/base/tests/test_scroll_selection_into_view.html has a test case that is selection happens in an iframe?

Duplicate of this bug: 1553610

I did a brief chat with masayuki about this (in the bus heading to Whister), and the conclusion is that we don't need to scroll selections across document boundaries. So probably we should set ScrollNoParentFrames in the case where we call ScrollFrameRectIntoView for selections.

It'd be nice to split the function into two, have ScrollFrameRectIntoViewCrossDoc (that returns void, since we can't know the answer synchronously with fission), and normal ScrollFrameRectIntoView (which implies ScrollNoParentFrames, and returns bool).

Priority: P3 → P2

This would be a bit cleaner once after we drop PresShell* argument from ScrollToShowRect in bug 1562503.

(In reply to Matt Woodrow (:mattwoodrow) from comment #3)

It'd be nice to split the function into two, have ScrollFrameRectIntoViewCrossDoc (that returns void, since we can't know the answer synchronously with fission), and normal ScrollFrameRectIntoView (which implies ScrollNoParentFrames, and returns bool).

Now I think we need three different ScrollFrameRectIntoViews;

  1. ScrollRectIntoView
  2. ScrollRectIntoViewSameOriginDoc for Element.scrollIntoView (bug 1561754 is to restrict it in the same origin)
  3. ScrollRectIntoViewCrossOriginDoc for 'Find in page'
Depends on: 1562503

Hiro, what are the next steps here? This sounds like something we should track for Fission, correct?

Flags: needinfo?(hikezoe.birchill)

Changing bug tile to reflect what we should do.

Flags: needinfo?(hikezoe.birchill)
Summary: Need a way to do PresShell::ScrollFrameRectIntoView across document boundaries for selection → Split PresShell::ScrollFrameRectIntoView into functions, one is for cross doc, the other is in process doc

I believe this should be Fission release blocker (M8).

Fission Milestone: --- → M8

Hiro, will you be working on this for Fission M8?

Flags: needinfo?(hikezoe.birchill)

Though I didn't have the plan to work on this, but if this needs an owner, I will manage to take time on this until M8.

Assignee: nobody → hikezoe.birchill
Status: NEW → ASSIGNED
Flags: needinfo?(hikezoe.birchill)

Is this bug for fixing an error in the fission case (e.g. scrolling works incorrectly), or is it a refactoring task? I'm having a bit of trouble telling from the bug description :-)

Flags: needinfo?(hikezoe.birchill)

This is a refactoring task.

Flags: needinfo?(hikezoe.birchill)
You need to log in before you can comment on or make changes to this bug.