From 2366cbc1a29cc71a625ee26d07a90fbc17239e5b Mon Sep 17 00:00:00 2001 From: David Bokan Date: Wed, 22 Nov 2023 15:09:41 -0500 Subject: [PATCH] Fix application of force-load-at-top (#243) Currently force-load-at-top is checked in the 'scroll to the fragment' steps. This is wrong since it shouldn't apply on same-document navigations and it should also affect history scroll restoration. Move this check to happen in the 'update document for history step application' where history scroll is restored and where we can differentiate between new- and same-document cases. Fixes #186 --- index.bs | 99 ++++++++++++++++++++++++++------- index.html | 158 ++++++++++++++++++++++++++++++++++++++++------------- 2 files changed, 200 insertions(+), 57 deletions(-) diff --git a/index.bs b/index.bs index d3a0686..ae308ab 100644 --- a/index.bs +++ b/index.bs @@ -845,10 +845,10 @@ prevent fragment scrolling if the force-load-at-top policy is enabled. Make the > What should be set as target if inside a shadow tree? > #190 > -> 4. Assert: |target| is an [=element=]. -> 5. Set |document|'s target element to |target|. -> 6. Run the ancestor details revealing algorithm on |target|. -> 7. Run the ancestor hidden-until-found revealing algorithm on |target|. +> 5. Assert: |target| is an [=element=]. +> 6. Set |document|'s target element to |target|. +> 7. Run the ancestor details revealing algorithm on |target|. +> 8. Run the ancestor hidden-until-found revealing algorithm on |target|. >
> These revealing algorithms currently wont work well since |target| could be an > ancestor or even the root document node. Issue @@ -856,26 +856,24 @@ prevent fragment scrolling if the force-load-at-top policy is enabled. Make the > restricting matches to `contain:style layout` blocks which would resolve this > problem. >
-> 8. Get the policy -> value for `force-load-at-top` for |document|. If the result is false: -> 1. [=scroll a target into view=], -> with target set to |scrollTarget|, behavior set to "auto", block set to "center", and -> inline set to "nearest". -> -> Implementations MAY avoid scrolling to the target if it is -> produced from a [=text directive=]. -> ->
-> force-load-at-top should be checked only when a new document is being -> loaded. -> #186 +> 9. Let |blockPosition| be "center" if |scrollTarget| is a [=range=], +> "start" otherwise. +>
+> Scrolling to a text directive centers it in the block flow direction. >
-> 9. Scroll target into view, with behavior set to "auto", block set to +> 10. Scroll target into view, with behavior set to "auto", block set to > "start", and inline set to "nearest". -> 10. Run the focusing steps for target, with the Document's viewport as the fallback target. +> +>
  • [=scroll a target into view=], +> with target set to |scrollTarget|, behavior set to "auto", +> block set to |blockPosition|, and inline set to "nearest". +> +> Implementations MAY avoid scrolling to the target if it is +> produced from a [=text directive=].
    • +> 11. Run the focusing steps for target, with the Document's viewport as the fallback target. >
    Implementation note: Blink doesn’t currently set focus for text > fragments, it probably should? TODO: file crbug.
    -> 11. Move the sequential focus navigation starting point to target. +> 12. Move the sequential focus navigation starting point to target. > >
    @@ -1255,6 +1253,67 @@ steps of the task queued in step 2: > document. > 3. Set document's [=document/allow text fragment scroll=] to false. +### Restricting Scroll on Load ### {#restricting-scroll-on-load} + +This section defines how the `force-load-at-top` policy is used to prevent all +types of scrolling when loading a new document, including but not limited to +text directives. + +ISSUE(WICG/scroll-to-text-fragment#242): Need to decide how `force-load-at-top` +interacts with the Navigation API. + +Amend the restore persisted state steps to take a new boolean +parameter which suppresses scroll restoration: + +> Monkeypatching [[HTML]]: +> +>
    +> To restore persisted state from a session history entry entry +> , and boolean |suppressScrolling|: +> +> 1. If entry's scroll restoration mode is "auto", |suppressScrolling| +> is false, and entry's document's relevant global object's navigation API's suppress +> normal scroll restoration during ongoing navigation is false, then restore scroll position +> data given entry. +> 2. ... + + +Amend the update document for history step application steps +to check the `force-load-at-top` policy and avoid scrolling in a new document +if it's set. + +> Monkeypatching [[HTML]]: +> +>
    +> 1. ... +>
  • Set document's history object's length to scriptHistoryLength.
    • +> 5. Let |scrollingBlockedInNewDocument| be the result of +> getting the policy +> value for `force-load-at-top` for |document|. +> 5. If documentsEntryChanged is true, then: +> 1. Let oldURL be document's latest entry's URL. +> 2. ... +>
  • If documentIsNew is false, then: +> 1. Update the navigation API entries for a same-document navigation given navigation, +> entry, and "traverse". +> 2. Fire an event named popstate... +> 3. Restore persisted state given entry and +> |suppressScrolling| set to false. +> 4. If oldURL's fragment is not equal to... +> 6. Otherwise, +> 1. Assert: entriesForNavigationAPI is given. +> 2. Restore persisted state given entry and +> |scrollingBlockedInNewDocument|. +> 3. Initialize the navigation API entries for a new document given navigation, +> entriesForNavigationAPI, and entry. +> 6. If documentIsNew is true, then: +> 1. If |scrollingBlockedInNewDocument| is false, try to scroll to +> the fragment for document. +> 2. At this point scripts may run for the newly-created document document. +> 7. Otherwise, if documentsEntryChanged is false and doNotReactivate is false, then: +> 1. ... +> +>
    • ## Navigating to a Text Fragment ## {#navigating-to-text-fragment} diff --git a/index.html b/index.html index ef2a342..6a90ac3 100644 --- a/index.html +++ b/index.html @@ -896,6 +896,7 @@

    Table of Contents

  • 3.5.2 Scroll On Navigation
  • 3.5.3 Search Timing
  • 3.5.4 Restricting the Text Fragment +
  • 3.5.5 Restricting Scroll on Load
  • 3.6 Navigating to a Text Fragment @@ -1443,7 +1444,7 @@

    update document for history step application:

    +

    In the definition of update document for history step application:

    Monkeypatching HTML § 7.4.6.2 Updating the document:

    @@ -1692,20 +1693,18 @@

    contain:style layout blocks which would resolve this problem.

  • -

    Get the policy - value for force-load-at-top for document. If the result is false:

    -
      -
    1. -

      scroll a target into view, - with target set to scrollTarget, behavior set to "auto", block set to "center", and inline set to "nearest".

      -

      Implementations MAY avoid scrolling to the target if it is - produced from a text directive.

      -
    -
    force-load-at-top should be checked only when a new document is being - loaded. #186
    +

    Let blockPosition be "center" if scrollTarget is a range, + "start" otherwise.

    +
    Scrolling to a text directive centers it in the block flow direction.
  • Scroll target into view, with behavior set to "auto", block set to "start", and inline set to "nearest". +
  • + scroll a target into view, + with target set to scrollTarget, behavior set to "auto", block set to blockPosition, and inline set to "nearest". +

    Implementations MAY avoid scrolling to the target if it is + produced from a text directive.

    +

  • Run the focusing steps for target, with the Document’s viewport as the fallback target.

    Implementation note: Blink doesn’t currently set focus for text @@ -2053,11 +2052,95 @@

    allow text fragment scroll to false.

    • +

    3.5.5. Restricting Scroll on Load

    +

    This section defines how the force-load-at-top policy is used to prevent all +types of scrolling when loading a new document, including but not limited to +text directives.

    +

    Need to decide how force-load-at-top interacts with the Navigation API. [Issue #WICG/scroll-to-text-fragment#242]

    +

    Amend the restore persisted state steps to take a new boolean +parameter which suppresses scroll restoration:

    +
    +

    Monkeypatching [HTML]:

    +
    + To restore persisted state from a session history entry entry , and boolean suppressScrolling: +
      +
    1. +

      If entry’s scroll restoration mode is "auto", suppressScrolling is false, and entry’s document’s relevant global object’s navigation API’s suppress +normal scroll restoration during ongoing navigation is false, then restore scroll position +data given entry.

      +
    2. +

      ...

      +
    +
    +
    +

    Amend the update document for history step application steps +to check the force-load-at-top policy and avoid scrolling in a new document +if it’s set.

    +
    +

    Monkeypatching [HTML]:

    +
    +
      +
    1. +

      ...

      +
    2. Set document’s history object’s length to scriptHistoryLength. +
    3. +

      Let scrollingBlockedInNewDocument be the result of getting the policy + value for force-load-at-top for document.

      +
    4. +

      If documentsEntryChanged is true, then:

      +
        +
      1. +

        Let oldURL be document’s latest entry’s URL.

        +
      2. +

        ...

        +
      3. + If documentIsNew is false, then: +
          +
        1. +

          Update the navigation API entries for a same-document navigation given navigation, + entry, and "traverse".

          +
        2. +

          Fire an event named popstate...

          +
        3. +

          Restore persisted state given entry and suppressScrolling set to false.

          +
        4. +

          If oldURL’s fragment is not equal to...

          +
        +
      4. +

        Otherwise,

        +
          +
        1. +

          Assert: entriesForNavigationAPI is given.

          +
        2. +

          Restore persisted state given entry and scrollingBlockedInNewDocument.

          +
        3. +

          Initialize the navigation API entries for a new document given navigation, + entriesForNavigationAPI, and entry.

          +
        +
      +
    5. +

      If documentIsNew is true, then:

      +
        +
      1. +

        If scrollingBlockedInNewDocument is false, try to scroll to + the fragment for document.

        +
      2. +

        At this point scripts may run for the newly-created document document.

        +
      +
    6. +

      Otherwise, if documentsEntryChanged is false and doNotReactivate is false, then:

      +
        +
      1. +

        ...

        +
      +
    +
    +

    3.6. Navigating to a Text Fragment

    The text fragment specification proposes an amendment to HTML § 7.4.2.3.3 Fragment navigations. In summary, if a text directive is present and a match is found in the page, the text fragment takes precedent over the element fragment as the indicated part. We amend the HTML Document’s -indicated part processing model to return a range, rather than an element, that will be scrolled into view.
    +indicated part processing model to return a range, rather than an element, that will be scrolled into view.
    • To find the first common ancestor of two nodes nodeA and nodeB, follow these steps: @@ -2083,7 +2166,7 @@

    3.6.1. Finding Ranges in a Document

    This section outlines several algorithms and definitions that specify how to - turn a full fragment directive string into a list of Ranges in the + turn a full fragment directive string into a list of Ranges in the document.

    At a high level, we take a fragment directive string that looks like this:

    text=prefix-,foo&unknown&text=bar,baz
@@ -2096,8 +2179,8 @@ 
    
-    
    If a directive successfully matches to text in the document, it returns a range indicating that match in the document. The invoke text directives steps are the high level API provided by this
-  section. These return a list of ranges that were matched
+    
    If a directive successfully matches to text in the document, it returns a range indicating that match in the document. The invoke text directives steps are the high level API provided by this
+  section. These return a list of ranges that were matched
   by the individual directive matching steps, in the order the directives were
   specified in the fragment directive string.
    
     
    If a directive was not matched, it does not add an item to the returned
@@ -2107,7 +2190,7 @@ 
    invoke text directives, given as input an ASCII string text directives and a Document document, run these steps: 
     
     This algorithm takes as input a text directives, that is the
   raw text of the fragment directive and the document over which it operates.
-  It returns a list of ranges that are to be visually
+  It returns a list of ranges that are to be visually
   indicated, the first of which will be scrolled into view (if the UA scrolls
   automatically). 
    
     
      
@@ -2119,7 +2202,7 @@ 
      strictly splitting the
 string text directives on "&".
      
      
    1. 
-      
      Let ranges be a list of ranges, initially empty.
      
+      
      Let ranges be a list of ranges, initially empty.
      
      
    2. 
       
      For each ASCII string directive of directives:
      
       
        
@@ -2143,12 +2226,12 @@ 
        
       This algorithm takes as input a successfully parsed text directive and a
-  document in which to search. It returns a range that points to the first
+  document in which to search. It returns a range that points to the first
   text passage within the document that matches the searched-for text and
   satisfies the surrounding context. Returns null if no such passage exists. 
      
        end can be null. If omitted, this is an "exact"
-  search and the returned range will contain a string exactly matching start. If end is
-  provided, this is a "range" search; the returned range will start with start and end with end. In the normative text below, we’ll call a
+  search and the returned range will contain a string exactly matching start. If end is
+  provided, this is a "range" search; the returned range will start with start and end with end. In the normative text below, we’ll call a
   text passage that matches the provided start and end, regardless of which mode we’re in, the
   "matching text".
        
      
        Either or both of prefix and suffix can be null, in which case context on that
@@ -2178,7 +2261,7 @@ 
        
     
          
      
        1. 
-      
          Let searchRange be a range with start (document, 0) and end (document, document’s length)
          
+      
          Let searchRange be a range with start (document, 0) and end (document, document’s length)
          
      
        2. 
       
          While searchRange is not collapsed:
          
       
            
@@ -2195,7 +2278,7 @@ 
            
           
            Set searchRange’s start to the first boundary point after prefixMatch’s start
            
          
          1. 
-          
            Let matchRange be a range whose start is prefixMatch’s end and end is searchRange’s end.
            
+          
            Let matchRange be a range whose start is prefixMatch’s end and end is searchRange’s end.
            
          
          2. 
           
            Advance matchRange’s start to the next non-whitespace position.
            
          
          3. 
@@ -2235,7 +2318,7 @@ 
            Set searchRange’s start to the first boundary point after potentialMatch’s start
            
         
          
        
        3. 
-        
          Let rangeEndSearchRange be a range whose start is potentialMatch’s end and whose end is searchRange’s end.
          
+        
          Let rangeEndSearchRange be a range whose start is potentialMatch’s end and whose end is searchRange’s end.
          
        
        4. 
         
          While rangeEndSearchRange is not collapsed:
          
         
            
@@ -2259,7 +2342,7 @@ 
            
           
            If parsedValues’s suffix is null, return potentialMatch.
            
          
          1. 
-          
            Let suffixRange be a range with start equal to potentialMatch’s end and end equal to searchRange’s end.
            
+          
            Let suffixRange be a range with start equal to potentialMatch’s end and end equal to searchRange’s end.
            
          
          2. 
           
            Advance suffixRange’s start to the next non-whitespace
 position.
            
@@ -2311,7 +2394,7 @@ 
            
    
    
            
-     To advance a range range’s start to the next
+     To advance a range range’s start to the next
 non-whitespace position follow the steps: 
     
              
      
            1. 
@@ -2358,16 +2441,16 @@ 
              
    
            
    
            
-     To find a string in range given a string query, a range searchRange, and booleans wordStartBounded and wordEndBounded,
+     To find a string in range given a string query, a range searchRange, and booleans wordStartBounded and wordEndBounded,
 run these steps: 
-    
             This algorithm will return a range that represents the first instance of
+    
             This algorithm will return a range that represents the first instance of
   the query text that is fully contained within searchRange, optionally
   restricting itself to matches that start and/or end at word boundaries (see § 3.6.2 Word Boundaries). Returns null if none is found. 
            
     
            
      
             The basic premise of this algorithm is to walk all searchable text nodes
     within a block, collecting them into a list. The list is then concatenated
     into a single string in which we can search, using the node list to
-    determine offsets with a node so we can return a range. 
            
+    determine offsets with a node so we can return a range. 
            
      
             Collection breaks when we hit a block node, e.g. searching over this tree: 
            
 
            <div>
   a<em>b</em>c<div>d</div>e
@@ -2430,7 +2513,7 @@ 
            Set curNode to the next node in shadow-including tree order.
            
         
          
        
        5. 
-        
          Run the find a range from a node list steps given query, searchRange, textNodeList, wordStartBounded and wordEndBounded as input. If the resulting range is not null, then return it.
          
+        
          Run the find a range from a node list steps given query, searchRange, textNodeList, wordStartBounded and wordEndBounded as input. If the resulting range is not null, then return it.
          
        
        6. 
         
          If curNode is null, then break.
          
        
        7. 
@@ -2478,7 +2561,7 @@ 
          
    
          
      To find a range from a node list given a search string queryString,
-a range searchRange, a list of Text nodes nodes, and booleans wordStartBounded and wordEndBounded, follow these steps: 
+a range searchRange, a list of Text nodes nodes, and booleans wordStartBounded and wordEndBounded, follow these steps: 
     
          
       Optionally, this will only return a match if the matched text begins and/or
   ends on a word boundary. For example: 
@@ -2552,7 +2635,7 @@ 
          
       
          Assert: start and end are non-null, valid boundary points in searchRange.
          
      
        8. 
-      
          Return a range with start start and end end.
          
+      
          Return a range with start start and end end.
    @@ -2727,7 +2810,7 @@

    Fragment-based scroll blocking from this policy is specified in an amendment to the scroll to the fragment algorithm in the § 3.6 Navigating to a Text Fragment section of this document.

    -

    History scroll restoration is blocked by amending the restore +

    History scroll restoration is blocked by amending the restore persisted state steps by inserting a new step after 2:

    1. @@ -3049,6 +3132,7 @@

      top-level browsing context
    2. transient activation
    3. try to scroll to the fragment +
    4. update document for history step application
    5. [INFRA] defines the following terms: @@ -3150,13 +3234,12 @@

      #89 proposes restricting matches to contain:style layout blocks which would resolve this problem. ↵

    -
    force-load-at-top should be checked only when a new document is being - loaded. #186 ↵
    Implementation note: Blink doesn’t currently set focus for text fragments, it probably should? TODO: file crbug. ↵
    This isn’t strictly true, Chrome allows this for same-origin initiators. Need to update the spec on this point. [Issue #WICG/scroll-to-text-fragment#240] ↵
    +
    Need to decide how force-load-at-top interacts with the Navigation API. [Issue #WICG/scroll-to-text-fragment#242] ↵
    data is not correct here since that’s the text data as it exists in the DOM. This algorithm means to run over the text as rendered (and then convert back @@ -3502,7 +3585,7 @@