Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Range.myGetExtensionRect: Region content starting with a comment-node results in an empty boundingRect #51

Open
RonaldTreur opened this issue Nov 22, 2020 · 0 comments

Comments

@RonaldTreur
Copy link
Contributor

RonaldTreur commented Nov 22, 2020

Note: This was only tested on Chrome v86

Some background

After adding content into a region, the polyfill's extractOverflowingContent method is being invoked if the content overflows the region. This method then will determine where to cut off the overflowing part, so that the remainder can then be rendered in the subsequent region.

It tries to be smart about it, by asking the browser to create a range near the end of the region. This can fail for a variety of reasons however (e.g. unsupported by Firefox, or text outside viewport).

It it does fail, it will follow a non-optimized route by creating a collapsed Range that is positioned at the very beginning of the region's content:

// move back into the region
r = r || document.createRange();
r.setStart(region, 0);
r.setEnd(region, 0);
dontOptimize=true;

The plan next is to move the range "to the right" bit by bit, until if falls outside of the region after which it knows where to cut the content off.

Before it performs this (iterative) process however, it begins with determining the bounding DOMRect of the previously created Range by invoking r.myGetExtensionRect():

var rect = r.myGetExtensionRect(); fixNullRect();

Unfortunately collapsed ranges lead to an "empty" (all values equal to 0) DOMRect in some browsers (e.g. Chrome). This is taken into account by the polyfill:

// if the value seems wrong... (some browsers don't like collapsed selections)
if(this.collapsed && rect.top===0 && rect.bottom===0) {

In that case (and since we are at the very beginning of the range) it will attempt to remedy this by first moving the range to the "right":

// let's move on char to the right
clone.myMoveTowardRight();
collapseToLeft=true;

This clone.myMoveTowardRight() results again a collapsed range (since this code is multi-purpose), but now the startOffset (and/or startContainer) will have changed. By invoking the following code, it then potentially "stretches" the range (back to where we started) which could result in a non-collapsed range:

// note: some browsers don't like selections
// that spans multiple containers, so we will
// iterate this process until we have one true
// char selected
clone.setStart(clone.endContainer, 0);

By performing a recursive call to r.myGetExtensionRect() next, a new bounding DOMRect will be determined. If it is still "empty" and still collapsed, it will move the range once more to the right and again stretch the range. This will continue until the DOMRect is no longer empty or the range is no longer collapsed.

What goes wrong

If this region's content starts with a comment-node, then moving the range to the right will put it to the right of the comment-node (as intended):

Next the resulting collapsed range will be updated to encompass this comment node by moving the start of the range back:

clone.setStart(clone.endContainer, 0);

Afterwards the recursive call will find (in Chrome at least) that this again results in an empty DOMRect (despite containing a comment node). Unfortunately it is no longer considered to be collapsed, resulting in this if-condition to fail:

if(this.collapsed && rect.top===0 && rect.bottom===0) {

This unfortunately leads to no more attempts being made to rectify this situation and the empty DOMRect is returned (in the end) to the extractOverflowingContent method.

Possible fix(es)

I have two rough ideas, though I am not 100% sure these do not break something else:

  1. Change the myMoveTowardRight method to always skip over comment-nodes, since they take up no space anyway
  2. Verify that the range contains only comment-nodes right after this line

    And keep executing that line until this is no longer the case

Final (important) note

Fortunately the polyfill has a failsafe built in, that will make sure the polyfill can (in most cases) still do its job:

var rect = r.myGetExtensionRect(); fixNullRect();

The fixNullRect function, that immediately follows the r.myGetExtensionRect()-call, will try to fix an empty boundingRect by creating a fake one with a less precise approximation of some of the properties needed by the code that follows.

So though I would not consider this "bug" to be a major issue, it might shed some light on one of the reasons why this fixNullRect function is needed to begin with.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant