Moving puck causes unresponsiveness #1802

enellis · 2024-06-05T12:52:06Z

Version: 1.19.0
System: Mobile Safari, iOS 17.4.1, iPhone 13 Pro

Description

After the update to version 1.19.0, when moving the puck on more complex pages (I noticed this while browsing NHKニュース), it frequently becomes unresponsive and takes a few seconds to become responsive again.

Troubleshooting

Initially, I suspected the switch from elementFromPoint() to elementsFromPoint() (#1773):

10ten-ja-reader/src/content/puck.ts

Lines 443 to 448 in 04233cf

    
           let target = 
        
             document 
        
               .elementsFromPoint(targetX, targetY) 
        
               // Ignore any element in the 10ten popup itself; we don't want 
        
               // the puck moon to hold the popup open like a mouse event does. 
        
               .find((target) => !target.closest('#tenten-ja-window')) || null;

Sure enough, when reverting the change to

let target = document.elementFromPoint(targetX, targetY);

the above-mentioned performance issues indeed do not occur.

Then I attempted to exploit the fact that elementFromPoint() ignores all elements with the pointer-events: none property to investigate whether really the call to elementsFromPoint() is the cause:

const container: HTMLElement | null | undefined = document
      .getElementById('tenten-ja-window')
      ?.shadowRoot?.querySelector('.container');

if (container)
  container.style.setProperty('pointer-events', 'none', 'important');

// See what we are pointing at
let target = document.elementFromPoint(targetX, targetY);
    
if (container) container.style.setProperty('pointer-events', 'all')

This code works as expected in that it scans the elements underneath the popup, but the exact same performance issues as with the original version occur, suggesting there might be another underlying cause for the freezing.

Has anyone experienced similar issues or any ideas as to what is causing this problem?

The text was updated successfully, but these errors were encountered:

birtles · 2024-06-06T00:26:33Z

cc @StarScape

I tried reproducing this in Firefox now but haven't succeeded. Have you noticed any particular content where it happens?

If I can repro in Firefox I can run the profiler on it. I haven't tried using Safari's performance tools but if we can get a profile of it happening on iPhone then we should be able to see where it is spending time.

birtles · 2024-06-06T00:48:01Z

I haven't had a chance to debug this yet but one possibility that comes to mind is we have this special logic for handling covered links:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 95 to 118 in 50fe928

    
           // Skip elements that are already visible 
        
           // 
        
           // We need special handling here to account for "covering links". 
        
           // 
        
           // Normally we can just check if the current element is invisible or not 
        
           // but for asahi.com we have a special case where it effectively makes the 
        
           // covering content invisible by setting the dimensions of a _child_ 
        
           // element to 1x1. 
        
           // 
        
           // To detect that case we check for a non-auto z-index since that has 
        
           // proven to be the most reliable indicator of this pattern. If we simply 
        
           // decide to treat the element as invisible whenever its bounding box 
        
           // doesn't line up, we'll run this too often and cause a performance 
        
           // regression when the the cursor is moving around empty space on the Web 
        
           // page. 
        
           // 
        
           // We only do this for the initial lookup for now because so far that's 
        
           // proved sufficient (and is probably cheaper than trying to perform this 
        
           // check on every element in the hit list). 
        
           const treatElementAsInvisible = 
        
             firstElement && getComputedStyle(element).zIndex !== 'auto'; 
        
           if (!treatElementAsInvisible && isVisible(element)) { 
        
             continue; 
        
           }

It's possible that by deliberately choosing a covered element we end up running that code in a situation it wasn't intended for.

Alternatively, if the NHK site is using shadow DOM we might be getting tripped up in some of our shadow DOM mirroring code.

In either case, there's a good chance its something in get-cursor-position.ts.

enellis · 2024-06-06T07:54:25Z

Call stack of a freeze (Sorry for the German: Startzeit: starting time, Gesamtdauer: duration):

enellis · 2024-06-06T14:28:10Z

The issue occurs every time you hover with the puck cursor over the 10ten popup window. To reproduce this, you have to move very quickly so the popup does not close when the puck cursor is in the gap between the puck cursor and the popup. This also occurs in Safari on macOS but I could not reproduce it in Chrome.

Also, this maybe explains why it's not reproducible in Firefox:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 554 to 557 in 50fe928

    
           // Unlike `document.caretPositionFromPoint` in Gecko, 
        
           // `document.caretRangeFromPoint` in Blink/WebKit doesn't dig into shadow DOM 
        
           // so we need to do it manually. 
        
           range = expandShadowDomInRange({ range, point });

It appears that the following code is quite slow. getComputedStyle() always returns ~650 properties, so this loop runs that many times for every node and child. Additionally, if you compare node.outerHTML and clone.outerHTML, you can see how the cloned node becomes very bloated as a result:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 755 to 764 in 50fe928

    
           const clone = node.cloneNode(false) as HTMLElement | SVGElement; 
        
           const cs = document.defaultView!.getComputedStyle(node); 
        
           for (let i = 0; i < cs.length; i++) { 
        
             const prop = cs.item(i); 
        
             clone.style.setProperty(prop, cs.getPropertyValue(prop)); 
        
           } 
        
           for (const child of node.childNodes) { 
        
             clone.appendChild(cloneNodeWithStyles(child)); 
        
           }

In any case we should consider returning early in expandShadowDomInRange() if the container id is tenten-ja-window because we never want to look up the contents of the 10ten popup window anyway. This would fix this issue and reduce the overhead of the computation if you happen to hover over the popup with the puck cursor.

StarScape · 2024-06-06T20:52:31Z

cc @StarScape

I tried reproducing this in Firefox now but haven't succeeded. Have you noticed any particular content where it happens?

If I can repro in Firefox I can run the profiler on it. I haven't tried using Safari's performance tools but if we can get a profile of it happening on iPhone then we should be able to see where it is spending time.

I came across this exact same bug a few days back but hadn’t filed a bug report yet. Likewise happened to me on iOS Safari on the NHK website. Haven’t noticed it anywhere else.

birtles · 2024-06-07T02:07:26Z

The issue occurs every time you hover with the puck cursor over the 10ten popup window. To reproduce this, you have to move very quickly so the popup does not close when the puck cursor is in the gap between the puck cursor and the popup. This also occurs in Safari on macOS but I could not reproduce it in Chrome.

Thanks so much for digging into this! I also tried reproducing in Chrome but could not.

Also, this maybe explains why it's not reproducible in Firefox:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 554 to 557 in 50fe928

// Unlike `document.caretPositionFromPoint` in Gecko,

// `document.caretRangeFromPoint` in Blink/WebKit doesn't dig into shadow DOM

// so we need to do it manually.

range = expandShadowDomInRange({ range, point });

Yes, that's right. Blink and Webkit currently only support caretRangeFromPoint but Blink is about to ship caretPositionFromPoint soon which will mean that only Safari continues using this code path.

It appears that the following code is quite slow. getComputedStyle() always returns ~650 properties, so this loop runs that many times for every node and child. Additionally, if you compare node.outerHTML and clone.outerHTML, you can see how the cloned node becomes very bloated as a result:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 755 to 764 in 50fe928

const clone = node.cloneNode(false) as HTMLElement | SVGElement;

const cs = document.defaultView!.getComputedStyle(node);

for (let i = 0; i < cs.length; i++) {

const prop = cs.item(i);

clone.style.setProperty(prop, cs.getPropertyValue(prop));

}

for (const child of node.childNodes) {

clone.appendChild(cloneNodeWithStyles(child));

}

The code could certainly be tweaked. We should fetch all the properties first before setting them but I don't think there's any way to avoid setting them all while maintaining exact equivalent layout.

In any case we should consider returning early in expandShadowDomInRange() if the container id is tenten-ja-window because we never want to look up the contents of the 10ten popup window anyway. This would fix this issue and reduce the overhead of the computation if you happen to hover over the popup with the puck cursor.

I think that would work but first I want to understand what is happening. I don't think it's a case of it just being slow to look up the popup's shadow DOM. I think there might be a more complicated interaction there like us trying to run the "covering link" logic but getting confused by the popup.

For example, perhaps we need to be ignoring the popup window in the call to elementsFromPoint here too:

10ten-ja-reader/src/content/get-text.ts

Line 65 in 6495d2f

const elements = [...new Set(document.elementsFromPoint(point.x, point.y))];

I'm just guessing, however, because unfortunately I don't have a Mac at home so I can't currently debug this. I have a Mac in the office but I'm only able to get there a couple of times a week at the moment.

birtles · 2024-06-07T02:10:02Z

I think that would work but first I want to understand what is happening. I don't think it's a case of it just being slow to look up the popup's shadow DOM. I think there might be a more complicated interaction there like us trying to run the "covering link" logic but getting confused by the popup.

One thing that would help in understanding this is to know if the call to getCursorPositionElement in the stack from #1802 (comment) is from this call site:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 59 to 62 in 6495d2f

    
           const initialResult = getCursorPositionForElement({ 
        
             point, 
        
             element: initialElements[0], 
        
           });

Or from this call site:

10ten-ja-reader/src/content/get-cursor-position.ts

Line 125 in 6495d2f

const result = getCursorPositionForElement({ point, element });

birtles · 2024-06-07T04:22:58Z

I'm still not sure when I'm going to get to a Mac next but I'd really like to ship a fix for this soon along with the pref to revert the font changes.

One suggestion is changing the other call site of elementsFromPoint in get-text.ts to exclude the popup there too per #1802 (comment).

Another one that occurred to me, however, is to revert the change in #1773 and change the following code:

10ten-ja-reader/src/content/content.ts

Lines 703 to 706 in cc0a4c4

    
           // Ignore mouse events on the popup window 
        
           if (isPopupWindowHostElem(event.target)) { 
        
             return; 
        
           }

to:

    // Ignore mouse events on the popup window
    if (isPopupWindowHostElem(event.target)) {
      // If the puck is over the popup window, hide the popup window
      if (isPuckPointerEvent(event)) {
        this.clearResult();
      }
      return;
    }

That would mean that instead of trying to look up content underneath the puck we'd initially close it and then on the next movement try to look up. I'm not sure if that's more or less useful, however.

If anyone has a chance to try out either of those suggestions that would be great! Thank you!

enellis · 2024-06-07T07:46:01Z

One thing that would help in understanding this is to know if the call to getCursorPositionElement in the stack from #1802 (comment) is from this call site:

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 59 to 62 in 6495d2f

const initialResult = getCursorPositionForElement({

point,

element: initialElements[0],

});

Or from this call site:

10ten-ja-reader/src/content/get-cursor-position.ts

Line 125 in 6495d2f

const result = getCursorPositionForElement({ point, element });

I logged every call to getCursorPositionElement (marked with their respective call site) that takes more than half a second to complete and I timed the inner body of cloneNodeWithStyles function, logging the outerHTML of nodes taking more than half a second to be cloned. This is the result of one single freeze:

[Log] Took 697 ms to clone node: 
[Log] <div class="tab-bar" lang="en"><ul class="tabs"><li class="tab" role="presentation" aria-selected="true"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><path d="M14,2H10.09a2.16,2.16,0,0,0-.71.12l-1.11.41a.83.83,0,0,1-.54,0L6.62,2.12A2.16,2.16,0,0,0,5.91,2H2A2,2,0,0,0,0,4v8a2,2,0,0,0,2.05,2H5.91a.76.76,0,0,1,.27.05l1.12.4a1.95,1.95,0,0,0,1.4,0L10.33,14l.84,0a.84.84,0,0,0,.71-.8c0-.67-.76-.69-.76-.69a5.17,5.17,0,0,0-1.25.12L9,13V4l.07,0,1.11-.4a.86.86,0,0,1,.27,0h3.27a.78.78,0,0,1,.78.78V9A.75.75,0,0,0,16,9V4A2,2,0,0,0,14,2ZM7,13l-.76-.33a1.85,1.85,0,0,0-.7-.13H2.28a.78.78,0,0,1-.78-.78V4.28a.78.78,0,0,1,.78-.78H5.54a.75.75,0,0,1,.26,0L6.92,4,7,4Z"></path><g fill="none" stroke="currentColor" stroke-linecap="round"><line x1="3" y1="7.5" x2="5.5" y2="7.5"></line><line x1="3" y1="5.5" x2="5.5" y2="5.5"></line><line x1="3" y1="9.5" x2="5.5" y2="9.5"></line><line x1="10.5" y1="7.5" x2="13" y2="7.5"></line><line x1="10.5" y1="5.5" x2="13" y2="5.5"></line><line x1="10.5" y1="9.5" x2="11.5" y2="9.5"></line></g><circle cx="14.5" cy="12.5" r="1.5"></circle></svg><span>Words</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M11,15H2a2,2,0,0,1-2-2V2A2,2,0,0,1,2,0H13a2,2,0,0,1,2,2v9a1,1,0,0,1-2,0V2H2V13h9a1,1,0,0,1,0,2Z"></path><path d="M8.5,7H5V6h5V7H9.5l-1,1H12V9H8v2a1,1,0,0,1-.24.71A1.15,1.15,0,0,1,7,12H6V11H7V9H3V8H7.5ZM8,4h4V6H11V5H4V6H3V4H7V3H8Z"></path></svg><span>Kanji</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M8,0A2.87,2.87,0,0,0,5,2.72v2.5A2.92,2.92,0,0,0,8,8a2.92,2.92,0,0,0,3-2.78V2.72A2.87,2.87,0,0,0,8,0Z"></path><path d="M13.91,11.71A5.09,5.09,0,0,0,9.45,9H5.09A5.18,5.18,0,0,0,0,14.25.74.74,0,0,0,.73,15h10.9a.74.74,0,0,0,.73-.75,1.49,1.49,0,0,1,1.09-1.45.75.75,0,0,0,.49-.43A.76.76,0,0,0,13.91,11.71Z"></path></svg><span>Names</span></button></li></ul><div class="pin"><button aria-label="Pin popup" title="Pin popup (Ctrl)" class="pin-button" type="button"><svg role="presentation" viewBox="0 0 24 24"><path d="m14 3 .593 1.833c.104.295.157.604.157.917v3.42l.666.236a2.759 2.759 0 0 1 1.834 2.591c0 .05 0 .197-.33.253a3.504 3.504 0 0 0-3.42 3.499c-.029.065-.283.251-.5.251h-1v4.75V16H8.904a2.156 2.156 0 0 1-2.154-2.154v-1.849a2.75 2.75 0 0 1 1.833-2.592l.667-.235V5.75c0-.313.053-.622.157-.916L10 3H8h8-2z" fill="none" stroke="currentColor"></path><circle cx="18" cy="16.5" r="1.5" fill="currentColor" stroke="none"></circle></svg></button></div><div class="settings"><button aria-label="Settings" title="Settings" class="settings-button" type="button"><svg viewBox="0 0 24 24"><circle cx="21.5" cy="21.5" r="1.5" fill="currentColor" stroke="none"></circle><circle cx="12" cy="12" r="4"></circle><path d="M10.48 3.28a2 2 0 003 0 2.05 2.05 0 013.57 1.48 2.05 2.05 0 002.15 2.15 2.05 2.05 0 011.48 3.57 2 2 0 000 3 2.05 2.05 0 01-1.48 3.57 2.05 2.05 0 00-2.15 2.15 2.05 2.05 0 01-3.57 1.48 2 2 0 00-3 0 2.05 2.05 0 01-3.57-1.48 2.05 2.05 0 00-2.15-2.15 2.05 2.05 0 01-1.48-3.57 2 2 0 000-3 2.05 2.05 0 011.48-3.57 2.05 2.05 0 002.15-2.15 2.05 2.05 0 013.57-1.48z"></path></svg></button></div><div class="close"><button aria-label="Close" title="Close (Esc)" class="close-button" type="button"><svg viewBox="0 0 24 24"><path d="M6 18L18 6M6 6l12 12"></path></svg></button></div></div>
[Log] Took 1092 ms to clone node: 
[Log] <div class="window theme-black bundled-fonts" data-tab-side="top"><div class="tab-bar" lang="en"><ul class="tabs"><li class="tab" role="presentation" aria-selected="true"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><path d="M14,2H10.09a2.16,2.16,0,0,0-.71.12l-1.11.41a.83.83,0,0,1-.54,0L6.62,2.12A2.16,2.16,0,0,0,5.91,2H2A2,2,0,0,0,0,4v8a2,2,0,0,0,2.05,2H5.91a.76.76,0,0,1,.27.05l1.12.4a1.95,1.95,0,0,0,1.4,0L10.33,14l.84,0a.84.84,0,0,0,.71-.8c0-.67-.76-.69-.76-.69a5.17,5.17,0,0,0-1.25.12L9,13V4l.07,0,1.11-.4a.86.86,0,0,1,.27,0h3.27a.78.78,0,0,1,.78.78V9A.75.75,0,0,0,16,9V4A2,2,0,0,0,14,2ZM7,13l-.76-.33a1.85,1.85,0,0,0-.7-.13H2.28a.78.78,0,0,1-.78-.78V4.28a.78.78,0,0,1,.78-.78H5.54a.75.75,0,0,1,.26,0L6.92,4,7,4Z"></path><g fill="none" stroke="currentColor" stroke-linecap="round"><line x1="3" y1="7.5" x2="5.5" y2="7.5"></line><line x1="3" y1="5.5" x2="5.5" y2="5.5"></line><line x1="3" y1="9.5" x2="5.5" y2="9.5"></line><line x1="10.5" y1="7.5" x2="13" y2="7.5"></line><line x1="10.5" y1="5.5" x2="13" y2="5.5"></line><line x1="10.5" y1="9.5" x2="11.5" y2="9.5"></line></g><circle cx="14.5" cy="12.5" r="1.5"></circle></svg><span>Words</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M11,15H2a2,2,0,0,1-2-2V2A2,2,0,0,1,2,0H13a2,2,0,0,1,2,2v9a1,1,0,0,1-2,0V2H2V13h9a1,1,0,0,1,0,2Z"></path><path d="M8.5,7H5V6h5V7H9.5l-1,1H12V9H8v2a1,1,0,0,1-.24.71A1.15,1.15,0,0,1,7,12H6V11H7V9H3V8H7.5ZM8,4h4V6H11V5H4V6H3V4H7V3H8Z"></path></svg><span>Kanji</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M8,0A2.87,2.87,0,0,0,5,2.72v2.5A2.92,2.92,0,0,0,8,8a2.92,2.92,0,0,0,3-2.78V2.72A2.87,2.87,0,0,0,8,0Z"></path><path d="M13.91,11.71A5.09,5.09,0,0,0,9.45,9H5.09A5.18,5.18,0,0,0,0,14.25.74.74,0,0,0,.73,15h10.9a.74.74,0,0,0,.73-.75,1.49,1.49,0,0,1,1.09-1.45.75.75,0,0,0,.49-.43A.76.76,0,0,0,13.91,11.71Z"></path></svg><span>Names</span></button></li></ul><div class="pin"><button aria-label="Pin popup" title="Pin popup (Ctrl)" class="pin-button" type="button"><svg role="presentation" viewBox="0 0 24 24"><path d="m14 3 .593 1.833c.104.295.157.604.157.917v3.42l.666.236a2.759 2.759 0 0 1 1.834 2.591c0 .05 0 .197-.33.253a3.504 3.504 0 0 0-3.42 3.499c-.029.065-.283.251-.5.251h-1v4.75V16H8.904a2.156 2.156 0 0 1-2.154-2.154v-1.849a2.75 2.75 0 0 1 1.833-2.592l.667-.235V5.75c0-.313.053-.622.157-.916L10 3H8h8-2z" fill="none" stroke="currentColor"></path><circle cx="18" cy="16.5" r="1.5" fill="currentColor" stroke="none"></circle></svg></button></div><div class="settings"><button aria-label="Settings" title="Settings" class="settings-button" type="button"><svg viewBox="0 0 24 24"><circle cx="21.5" cy="21.5" r="1.5" fill="currentColor" stroke="none"></circle><circle cx="12" cy="12" r="4"></circle><path d="M10.48 3.28a2 2 0 003 0 2.05 2.05 0 013.57 1.48 2.05 2.05 0 002.15 2.15 2.05 2.05 0 011.48 3.57 2 2 0 000 3 2.05 2.05 0 01-1.48 3.57 2.05 2.05 0 00-2.15 2.15 2.05 2.05 0 01-3.57 1.48 2 2 0 00-3 0 2.05 2.05 0 01-3.57-1.48 2.05 2.05 0 00-2.15-2.15 2.05 2.05 0 01-1.48-3.57 2 2 0 000-3 2.05 2.05 0 011.48-3.57 2.05 2.05 0 002.15-2.15 2.05 2.05 0 013.57-1.48z"></path></svg></button></div><div class="close"><button aria-label="Close" title="Close (Esc)" class="close-button" type="button"><svg viewBox="0 0 24 24"><path d="M6 18L18 6M6 6l12 12"></path></svg></button></div></div><div class="content"><div class="expandable" style="height: 114.402176px; scroll-snap-type: y mandatory;"><div class="wordlist entry-data"><div class="entry"><div><span class="w-kanji" lang="ja">省</span><span class="w-kana" lang="ja"><span class="w-binary"><span class="h-l">しょ</span><span class="l">う</span></span></span></div><div class="w-def"><ol><li class="foreign" lang="en"><span class="w-pos tag" lang="en">noun</span><span class="w-pos tag" lang="en">n-suf</span>ministry; department</li><li class="foreign" lang="en"><span class="w-pos tag" lang="en">noun</span><span class="w-pos tag" lang="en">n-suf</span>province (of China)</li><li class="foreign" lang="en"><span class="w-pos tag" lang="en">n-pref</span>saving; conserving</li></ol></div></div></div><button class="expand-button" title="Expand (x)" type="button" style="display: none;"><svg class="icon" viewBox="0 0 24 24" role="presentation"><path fill="currentColor" d="M21 6c1.7 0 2.6 2 1.4 3.2L13.5 20c-.7.9-2.3.9-3 0L1.6 9.2C.4 8 1.3 6 3 6h18z"></path></svg></button></div></div></div>
[Log] Took 1114 ms for getCursorPositionForElement (Line 59) to finish.
[Log] Took 696 ms to clone node:
[Log] <div class="tab-bar" lang="en"><ul class="tabs"><li class="tab" role="presentation" aria-selected="true"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><path d="M14,2H10.09a2.16,2.16,0,0,0-.71.12l-1.11.41a.83.83,0,0,1-.54,0L6.62,2.12A2.16,2.16,0,0,0,5.91,2H2A2,2,0,0,0,0,4v8a2,2,0,0,0,2.05,2H5.91a.76.76,0,0,1,.27.05l1.12.4a1.95,1.95,0,0,0,1.4,0L10.33,14l.84,0a.84.84,0,0,0,.71-.8c0-.67-.76-.69-.76-.69a5.17,5.17,0,0,0-1.25.12L9,13V4l.07,0,1.11-.4a.86.86,0,0,1,.27,0h3.27a.78.78,0,0,1,.78.78V9A.75.75,0,0,0,16,9V4A2,2,0,0,0,14,2ZM7,13l-.76-.33a1.85,1.85,0,0,0-.7-.13H2.28a.78.78,0,0,1-.78-.78V4.28a.78.78,0,0,1,.78-.78H5.54a.75.75,0,0,1,.26,0L6.92,4,7,4Z"></path><g fill="none" stroke="currentColor" stroke-linecap="round"><line x1="3" y1="7.5" x2="5.5" y2="7.5"></line><line x1="3" y1="5.5" x2="5.5" y2="5.5"></line><line x1="3" y1="9.5" x2="5.5" y2="9.5"></line><line x1="10.5" y1="7.5" x2="13" y2="7.5"></line><line x1="10.5" y1="5.5" x2="13" y2="5.5"></line><line x1="10.5" y1="9.5" x2="11.5" y2="9.5"></line></g><circle cx="14.5" cy="12.5" r="1.5"></circle></svg><span>Words</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M11,15H2a2,2,0,0,1-2-2V2A2,2,0,0,1,2,0H13a2,2,0,0,1,2,2v9a1,1,0,0,1-2,0V2H2V13h9a1,1,0,0,1,0,2Z"></path><path d="M8.5,7H5V6h5V7H9.5l-1,1H12V9H8v2a1,1,0,0,1-.24.71A1.15,1.15,0,0,1,7,12H6V11H7V9H3V8H7.5ZM8,4h4V6H11V5H4V6H3V4H7V3H8Z"></path></svg><span>Kanji</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M8,0A2.87,2.87,0,0,0,5,2.72v2.5A2.92,2.92,0,0,0,8,8a2.92,2.92,0,0,0,3-2.78V2.72A2.87,2.87,0,0,0,8,0Z"></path><path d="M13.91,11.71A5.09,5.09,0,0,0,9.45,9H5.09A5.18,5.18,0,0,0,0,14.25.74.74,0,0,0,.73,15h10.9a.74.74,0,0,0,.73-.75,1.49,1.49,0,0,1,1.09-1.45.75.75,0,0,0,.49-.43A.76.76,0,0,0,13.91,11.71Z"></path></svg><span>Names</span></button></li></ul><div class="pin"><button aria-label="Pin popup" title="Pin popup (Ctrl)" class="pin-button" type="button"><svg role="presentation" viewBox="0 0 24 24"><path d="m14 3 .593 1.833c.104.295.157.604.157.917v3.42l.666.236a2.759 2.759 0 0 1 1.834 2.591c0 .05 0 .197-.33.253a3.504 3.504 0 0 0-3.42 3.499c-.029.065-.283.251-.5.251h-1v4.75V16H8.904a2.156 2.156 0 0 1-2.154-2.154v-1.849a2.75 2.75 0 0 1 1.833-2.592l.667-.235V5.75c0-.313.053-.622.157-.916L10 3H8h8-2z" fill="none" stroke="currentColor"></path><circle cx="18" cy="16.5" r="1.5" fill="currentColor" stroke="none"></circle></svg></button></div><div class="settings"><button aria-label="Settings" title="Settings" class="settings-button" type="button"><svg viewBox="0 0 24 24"><circle cx="21.5" cy="21.5" r="1.5" fill="currentColor" stroke="none"></circle><circle cx="12" cy="12" r="4"></circle><path d="M10.48 3.28a2 2 0 003 0 2.05 2.05 0 013.57 1.48 2.05 2.05 0 002.15 2.15 2.05 2.05 0 011.48 3.57 2 2 0 000 3 2.05 2.05 0 01-1.48 3.57 2.05 2.05 0 00-2.15 2.15 2.05 2.05 0 01-3.57 1.48 2 2 0 00-3 0 2.05 2.05 0 01-3.57-1.48 2.05 2.05 0 00-2.15-2.15 2.05 2.05 0 01-1.48-3.57 2 2 0 000-3 2.05 2.05 0 011.48-3.57 2.05 2.05 0 002.15-2.15 2.05 2.05 0 013.57-1.48z"></path></svg></button></div><div class="close"><button aria-label="Close" title="Close (Esc)" class="close-button" type="button"><svg viewBox="0 0 24 24"><path d="M6 18L18 6M6 6l12 12"></path></svg></button></div></div>
[Log] Took 1091 ms to clone node:
[Log] <div class="window theme-black bundled-fonts" data-tab-side="top"><div class="tab-bar" lang="en"><ul class="tabs"><li class="tab" role="presentation" aria-selected="true"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><path d="M14,2H10.09a2.16,2.16,0,0,0-.71.12l-1.11.41a.83.83,0,0,1-.54,0L6.62,2.12A2.16,2.16,0,0,0,5.91,2H2A2,2,0,0,0,0,4v8a2,2,0,0,0,2.05,2H5.91a.76.76,0,0,1,.27.05l1.12.4a1.95,1.95,0,0,0,1.4,0L10.33,14l.84,0a.84.84,0,0,0,.71-.8c0-.67-.76-.69-.76-.69a5.17,5.17,0,0,0-1.25.12L9,13V4l.07,0,1.11-.4a.86.86,0,0,1,.27,0h3.27a.78.78,0,0,1,.78.78V9A.75.75,0,0,0,16,9V4A2,2,0,0,0,14,2ZM7,13l-.76-.33a1.85,1.85,0,0,0-.7-.13H2.28a.78.78,0,0,1-.78-.78V4.28a.78.78,0,0,1,.78-.78H5.54a.75.75,0,0,1,.26,0L6.92,4,7,4Z"></path><g fill="none" stroke="currentColor" stroke-linecap="round"><line x1="3" y1="7.5" x2="5.5" y2="7.5"></line><line x1="3" y1="5.5" x2="5.5" y2="5.5"></line><line x1="3" y1="9.5" x2="5.5" y2="9.5"></line><line x1="10.5" y1="7.5" x2="13" y2="7.5"></line><line x1="10.5" y1="5.5" x2="13" y2="5.5"></line><line x1="10.5" y1="9.5" x2="11.5" y2="9.5"></line></g><circle cx="14.5" cy="12.5" r="1.5"></circle></svg><span>Words</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M11,15H2a2,2,0,0,1-2-2V2A2,2,0,0,1,2,0H13a2,2,0,0,1,2,2v9a1,1,0,0,1-2,0V2H2V13h9a1,1,0,0,1,0,2Z"></path><path d="M8.5,7H5V6h5V7H9.5l-1,1H12V9H8v2a1,1,0,0,1-.24.71A1.15,1.15,0,0,1,7,12H6V11H7V9H3V8H7.5ZM8,4h4V6H11V5H4V6H3V4H7V3H8Z"></path></svg><span>Kanji</span></button></li><li class="tab" role="presentation"><button><svg viewBox="0 0 16 16" role="presentation" class="icon"><circle cx="14.5" cy="14.5" r="1.5"></circle><path d="M8,0A2.87,2.87,0,0,0,5,2.72v2.5A2.92,2.92,0,0,0,8,8a2.92,2.92,0,0,0,3-2.78V2.72A2.87,2.87,0,0,0,8,0Z"></path><path d="M13.91,11.71A5.09,5.09,0,0,0,9.45,9H5.09A5.18,5.18,0,0,0,0,14.25.74.74,0,0,0,.73,15h10.9a.74.74,0,0,0,.73-.75,1.49,1.49,0,0,1,1.09-1.45.75.75,0,0,0,.49-.43A.76.76,0,0,0,13.91,11.71Z"></path></svg><span>Names</span></button></li></ul><div class="pin"><button aria-label="Pin popup" title="Pin popup (Ctrl)" class="pin-button" type="button"><svg role="presentation" viewBox="0 0 24 24"><path d="m14 3 .593 1.833c.104.295.157.604.157.917v3.42l.666.236a2.759 2.759 0 0 1 1.834 2.591c0 .05 0 .197-.33.253a3.504 3.504 0 0 0-3.42 3.499c-.029.065-.283.251-.5.251h-1v4.75V16H8.904a2.156 2.156 0 0 1-2.154-2.154v-1.849a2.75 2.75 0 0 1 1.833-2.592l.667-.235V5.75c0-.313.053-.622.157-.916L10 3H8h8-2z" fill="none" stroke="currentColor"></path><circle cx="18" cy="16.5" r="1.5" fill="currentColor" stroke="none"></circle></svg></button></div><div class="settings"><button aria-label="Settings" title="Settings" class="settings-button" type="button"><svg viewBox="0 0 24 24"><circle cx="21.5" cy="21.5" r="1.5" fill="currentColor" stroke="none"></circle><circle cx="12" cy="12" r="4"></circle><path d="M10.48 3.28a2 2 0 003 0 2.05 2.05 0 013.57 1.48 2.05 2.05 0 002.15 2.15 2.05 2.05 0 011.48 3.57 2 2 0 000 3 2.05 2.05 0 01-1.48 3.57 2.05 2.05 0 00-2.15 2.15 2.05 2.05 0 01-3.57 1.48 2 2 0 00-3 0 2.05 2.05 0 01-3.57-1.48 2.05 2.05 0 00-2.15-2.15 2.05 2.05 0 01-1.48-3.57 2 2 0 000-3 2.05 2.05 0 011.48-3.57 2.05 2.05 0 002.15-2.15 2.05 2.05 0 013.57-1.48z"></path></svg></button></div><div class="close"><button aria-label="Close" title="Close (Esc)" class="close-button" type="button"><svg viewBox="0 0 24 24"><path d="M6 18L18 6M6 6l12 12"></path></svg></button></div></div><div class="content"><div class="expandable" style="height: 114.402176px; scroll-snap-type: y mandatory;"><div class="wordlist entry-data"><div class="entry"><div><span class="w-kanji" lang="ja">省</span><span class="w-kana" lang="ja"><span class="w-binary"><span class="h-l">しょ</span><span class="l">う</span></span></span></div><div class="w-def"><ol><li class="foreign" lang="en"><span class="w-pos tag" lang="en">noun</span><span class="w-pos tag" lang="en">n-suf</span>ministry; department</li><li class="foreign" lang="en"><span class="w-pos tag" lang="en">noun</span><span class="w-pos tag" lang="en">n-suf</span>province (of China)</li><li class="foreign" lang="en"><span class="w-pos tag" lang="en">n-pref</span>saving; conserving</li></ol></div></div></div><button class="expand-button" title="Expand (x)" type="button" style="display: none;"><svg class="icon" viewBox="0 0 24 24" role="presentation"><path fill="currentColor" d="M21 6c1.7 0 2.6 2 1.4 3.2L13.5 20c-.7.9-2.3.9-3 0L1.6 9.2C.4 8 1.3 6 3 6h18z"></path></svg></button></div></div></div>
[Log] Took 1111 ms for getCursorPositionForElement (Line 125) to finish.

enellis · 2024-06-07T09:16:31Z

For a short overview, I quickly tried out all of the current suggestions:

In any case we should consider returning early in expandShadowDomInRange() if the container id is tenten-ja-window because we never want to look up the contents of the 10ten popup window anyway.

Seems to work.

One suggestion is changing the other call site of elementsFromPoint in get-text.ts to exclude the popup there too per #1802 (comment).

I tried the following but the issue still persists:

  // First fetch the hit elements (dropping duplicates)
  const elements = [
    ...new Set(document.elementsFromPoint(point.x, point.y)),
  ].filter((target) => !target.closest('#tenten-ja-window'));

EDIT 1: I didn't really understand this behavior as I expected this to work, so I dug a little bit deeper. As last element elementsFromPoint returns the whole HTML body where the popup is a child from. So I changed the code to

  // First fetch the hit elements (dropping duplicates)
  const elements = [
    ...new Set(document.elementsFromPoint(point.x, point.y)),
  ].filter(
    (target) =>
      target.id !== 'tenten-ja-window' &&
      !target.querySelector('#tenten-ja-window')
  );

but the issue somehow still persists.

EDIT 2:
I think it is because the following code is run in any case for every element. The call to caretRangeFromPoint will always return a Range including the 10ten window if it is present at that point, so the popup gets mirrored in expandShadowDomInRange which takes a lot of time to compute.

10ten-ja-reader/src/content/get-cursor-position.ts

Lines 545 to 557 in cc0a4c4

    
           let range = document.caretRangeFromPoint(point.x, point.y); 
        
           if ( 
        
             !range || 
        
             (limitToDescendants && !element.contains(range.startContainer)) 
        
           ) { 
        
             return null; 
        
           } 
        
           // Unlike `document.caretPositionFromPoint` in Gecko, 
        
           // `document.caretRangeFromPoint` in Blink/WebKit doesn't dig into shadow DOM 
        
           // so we need to do it manually. 
        
           range = expandShadowDomInRange({ range, point });

Another one that occurred to me, however, is to revert the change in #1773 and change the following code:

10ten-ja-reader/src/content/content.ts

Lines 703 to 706 in cc0a4c4

// Ignore mouse events on the popup window

if (isPopupWindowHostElem(event.target)) {

return;

}

to:
    // Ignore mouse events on the popup window
    if (isPopupWindowHostElem(event.target)) {
      // If the puck is over the popup window, hide the popup window
      if (isPuckPointerEvent(event)) {
        this.clearResult();
      }
      return;
    }

The popup is closed most of the time even when it shouldn't be.

StarScape · 2024-06-07T22:14:15Z

I'm still not sure when I'm going to get to a Mac next but I'd really like to ship a fix for this soon along with the pref to revert the font changes.

Seems like you and @enellis have this in hand already, but on the off chance either of you need help from someone else with a Mac dev environment, just drop me an @, I'll be happy to pitch in a hand.

Fixes #1802.

birtles · 2024-06-08T05:41:13Z

For a short overview, I quickly tried out all of the current suggestions:

@enellis Thanks that was really useful investigation. I managed to get into the office today and could reproduce the issue with results like you reported.

I've gone ahead and added a fix along the lines you suggested. This seems to work for me. If either you or @StarScape have a chance to confirm the fix that would be great.

Fixes #1802.

birtles · 2024-06-08T05:42:00Z

Seems like you and @enellis have this in hand already, but on the off chance either of you need help from someone else with a Mac dev environment, just drop me an @, I'll be happy to pitch in a hand.

Thanks! I've just merged a fix for this into main so if you have a chance to test it out that would be very helpful!

birtles · 2024-06-08T06:24:36Z

For what it's worth, it looks like the issue really was just the performance of mirroring the shadow DOM elements. Specifically, it looks like WebKit doesn't optimize style flushes very effectively. Even when settings styles on an orphaned element it appears to invalidate styles. Hopefully this will be ultimately fixed when Safari implements caretPositionFromPoint one day.

enellis · 2024-06-08T07:26:16Z

I can confirm that it works on both iOS and macOS. Thank you!

birtles · 2024-06-08T07:57:26Z

I can confirm that it works on both iOS and macOS. Thank you!

Great, thank you! I've submitted the 1.19.1 release to the different app stores now so hopefully this is fixed before too long. Thanks again for all your help!

birtles mentioned this issue Jun 6, 2024

Revisit bundled font #1799

Closed

enellis changed the title ~~Moving pluck causes unresponsiveness~~ Moving puck causes unresponsiveness Jun 6, 2024

birtles added a commit that referenced this issue Jun 8, 2024

fix: fix puck lookup stalling on iOS Safari

65b12e1

Fixes #1802.

birtles mentioned this issue Jun 8, 2024

fix: fix puck lookup stalling on iOS Safari #1810

Merged

birtles closed this as completed in #1810 Jun 8, 2024

birtles added a commit that referenced this issue Jun 8, 2024

fix: fix puck lookup stalling on iOS Safari

7c8db17

Fixes #1802.

birtles mentioned this issue Jun 8, 2024

fix: prevent rendering of verb class tags on proverbs #1809

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moving puck causes unresponsiveness #1802

Moving puck causes unresponsiveness #1802

enellis commented Jun 5, 2024 •

edited

Loading

birtles commented Jun 6, 2024

birtles commented Jun 6, 2024

enellis commented Jun 6, 2024

enellis commented Jun 6, 2024 •

edited

Loading

StarScape commented Jun 6, 2024 •

edited

Loading

birtles commented Jun 7, 2024

birtles commented Jun 7, 2024

birtles commented Jun 7, 2024

enellis commented Jun 7, 2024 •

edited

Loading

enellis commented Jun 7, 2024 •

edited

Loading

StarScape commented Jun 7, 2024

birtles commented Jun 8, 2024

birtles commented Jun 8, 2024

birtles commented Jun 8, 2024

enellis commented Jun 8, 2024

birtles commented Jun 8, 2024

Moving puck causes unresponsiveness #1802

Moving puck causes unresponsiveness #1802

Comments

enellis commented Jun 5, 2024 • edited Loading

Description

Troubleshooting

birtles commented Jun 6, 2024

birtles commented Jun 6, 2024

enellis commented Jun 6, 2024

enellis commented Jun 6, 2024 • edited Loading

StarScape commented Jun 6, 2024 • edited Loading

birtles commented Jun 7, 2024

birtles commented Jun 7, 2024

birtles commented Jun 7, 2024

enellis commented Jun 7, 2024 • edited Loading

enellis commented Jun 7, 2024 • edited Loading

StarScape commented Jun 7, 2024

birtles commented Jun 8, 2024

birtles commented Jun 8, 2024

birtles commented Jun 8, 2024

enellis commented Jun 8, 2024

birtles commented Jun 8, 2024

enellis commented Jun 5, 2024 •

edited

Loading

enellis commented Jun 6, 2024 •

edited

Loading

StarScape commented Jun 6, 2024 •

edited

Loading

enellis commented Jun 7, 2024 •

edited

Loading

enellis commented Jun 7, 2024 •

edited

Loading