Safari version: 11.0.2 Here's the html to repro this: ``` <html> <head> </head> <body> <div> Something goes here! <br> Now more goes here. </div> <script> let r = new Range(); let node = document.getElementsByTagName('div')[0].childNodes[2]; r.setStart(node, 0); r.setEnd(node, 15); let rect = r.getBoundingClientRect(); alert(JSON.stringify(rect)); </script> </body> </html> ``` I get a 0,0,0,0 rect. I can repro 100% of the time on one laptop, but can't at all on another until I turn on "Responsive Design Mode" and then I can repro for that tab again. This happens reliably on 3 different iPhone with iOS 11.2. Does not happen on chrome or firefox. I also can't find document that would explain why this is happening. If this is expected, then would you have a suggestion for how to get a rectangle around part of that 2nd text node? This only happens when there's a <br> followed by a text node. The only maybe related bug I've found was https://bugs.webkit.org/show_bug.cgi?id=38394, but that one has a more complicated repro procedure. I don't know the Webkit codebase at all, but my guess is that this function: https://github.com/WebKit/webkit/blob/master/Source/WebCore/rendering/SimpleLineLayoutResolver.cpp#L205 returns an empty range, making the body of this loop https://github.com/WebKit/webkit/blob/7bdbecb7e435a102c10bc4ed27986720ae84ce95/Source/WebCore/rendering/SimpleLineLayoutFunctions.cpp#L233 never execute, and so the whole thing returns an empty collection of quads. The reason is simply that it seems like all codepaths inside the for loop add something to the vector of quads. Just a thought.
I've been able to repro this with 2 text nodes next to each other under a div: ``` <html> <head> </head> <body> <script> let d = document.createElement('div'); d.appendChild(document.createTextNode("Testing something")); d.appendChild(document.createTextNode("And another thing here")); document.body.appendChild(d); let r = new Range(); let node = document.getElementsByTagName('div')[0].childNodes[1]; r.setStart(node, 0); r.setEnd(node, 15); let rect = r.getBoundingClientRect(); alert(JSON.stringify(rect)); </script> </body> </html> ``` This will popup a 0,0,0,0 rect.
Here is a more illustrative example that might help. I highlight 3 ranges - one from the first text node's to the 2nd text node's 15th char - one from the first text node's to the 3rd text node's 0th char - one from the first text node's to the 3rd text node's 15th char The first range goes to the 0th char of the 2nd text node instead of the 15th char. The last two ranges return the same bounding rect. ``` <html> <head> </head> <body> <script> function drawRect(rect) { let d = document.createElement("div"); d.style.backgroundColor = "red"; d.style.left = rect.left + "px"; d.style.top = rect.top + "px"; d.style.width = rect.width + "px"; d.style.height = rect.height + "px"; d.style.position = "absolute"; d.style.opacity = 0.5; document.body.appendChild(d); } let d = document.createElement('div'); d.appendChild(document.createTextNode("Testing something")); d.appendChild(document.createTextNode("Testing and things here")); d.appendChild(document.createTextNode("And another thing here")); document.body.appendChild(d); { let r = new Range(); r.setStart(document.getElementsByTagName('div')[0].childNodes[0], 0); r.setEnd(document.getElementsByTagName('div')[0].childNodes[1], 15); drawRect(r.getBoundingClientRect()); } { let r = new Range(); r.setStart(document.getElementsByTagName('div')[0].childNodes[0], 0); r.setEnd(document.getElementsByTagName('div')[0].childNodes[2], 0); drawRect(r.getBoundingClientRect()); } { let r = new Range(); r.setStart(document.getElementsByTagName('div')[0].childNodes[0], 0); r.setEnd(document.getElementsByTagName('div')[0].childNodes[2], 15); drawRect(r.getBoundingClientRect()); } </script> </body> </html> ```
*** Bug 192495 has been marked as a duplicate of this bug. ***
Created attachment 363836 [details] Fix Range end offsets This patch changes the way the end offsets are calculated. It (correctly) uses the local-end offsets rather than using the end-offset from within the node. The existing code was mostly working before because contained nodes use maxInt to signify fully contained so this could only effect the last node.
Created attachment 363840 [details] Fix Range end offsets Updated patch to remove some changes that didn't need to be made.
Comment on attachment 363840 [details] Fix Range end offsets View in context: https://bugs.webkit.org/attachment.cgi?id=363840&action=review > LayoutTests/ChangeLog:3 > + Test getBoundingClientRect with a Range that contains a line break. Not conformant to our style. This line should be bug title verbatim. Description goes under reviewed by. > Source/WebCore/ChangeLog:3 > + Correct height of RenderText in a block element when used in a Range Ditto.
Created attachment 363861 [details] Fix Range end offsets updates patch Changelog
I first started seeing this bug sometime around iOS11; It seems to have been introduced in https://trac.webkit.org/changeset/212693/webkit I have worked around the bug by wrapping text in a span-tag when using Range.getBoundingClientRect(). Starting a Range in an in-line-level element live a span(rather than block-element like a div) bypasses the buggy code by disabling the use of simpleLineLayout. Using this span-wrap-hack has many side-effects and isn't ideal(dom modification, CSS selector breakage, etc). It's likely possible that some of the simpleLineLayout bypass-code can also be removed. e.g. > if (simpleLineLayout() && parent()->firstChild() == parent()->lastChild()) {
> if (simpleLineLayout() && parent()->firstChild() == parent()->lastChild()) { comes from https://trac.webkit.org/changeset/215054/webkit which appears to have been a workaround for this same bug. I'm not sure if it's best to revert that in the same patch as fixing the Range bug, or would it be better to do that separately?
This needs to be reviewed by Simon or Zalan.
@zalan@apple.com You had reviewed an earlier patch I had for this in https://bugs.webkit.org/show_bug.cgi?id=192495 (That patch was more of a work-around, less a fix) Digging in to find the actual issue I realized that my bug(192495) was a dupe of this one, so I moved the fix for the actual issue here.
Filed rdar://49352053
Antti/Zalan: could either one of you review this??
Friendly ping
This bug makes reliably measuring CJK content height nearly impossible and measuring the height of English content extremely slower(e.g. transforming English-language DOM so that it never hits a <br> that activates this condition is a slow operation).
r=me
Comment on attachment 363861 [details] Fix Range end offsets Clearing flags on attachment: 363861 Committed r245534: <https://trac.webkit.org/changeset/245534>
All reviewed patches have been landed. Closing bug.
<rdar://problem/50958290>