Bug 134466 - REGRESSION (WK2): Weird selection behavior on autos.yahoo.com, en.wikipedia.org
Summary: REGRESSION (WK2): Weird selection behavior on autos.yahoo.com, en.wikipedia.org
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Enrica Casucci
Keywords: InRadar
Depends on:
Reported: 2014-06-30 15:57 PDT by Enrica Casucci
Modified: 2014-06-30 17:28 PDT (History)
5 users (show)

See Also:

Patch (8.24 KB, patch)
2014-06-30 16:44 PDT, Enrica Casucci
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Enrica Casucci 2014-06-30 15:57:18 PDT
This bugs track a number of issues in block selections in WK2:
- incorrect block detections shrinking a block selection
- crashes when extending a selection across frame boundaries
- bugs in switching from text to block selection.

Comment 1 Enrica Casucci 2014-06-30 16:44:34 PDT
Created attachment 234117 [details]
Comment 2 Benjamin Poulain 2014-06-30 17:00:17 PDT
Comment on attachment 234117 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=234117&action=review

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:760
> +    // Make sure the block is selectable.

Probably not useful.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:764
> +    if (renderer->childrenInline() && (renderer->isRenderBlock() && toRenderBlock(renderer)->inlineElementContinuation() == nil) && !renderer->isTable()) {

This "== nil" probably comes from UIKit, you can update this to the WebKit style.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:770
> +    // If all we could find is a block whose height is veriy close to the height

Typo: "veriy"

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:772
> +    const float factor = .97;

Maybe find a better name for this?

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:774
> +    boundingRectInScrollViewCoordinates.scale(m_page->pageScaleFactor());

This is unlikely correct. The absoluteBoundingBoxRect and unobscuredContentRect should be in the same coordinate system already.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:776
> +    if (boundingRectInScrollViewCoordinates.height() > m_page->mainFrame().view()->unobscuredContentRect().height() * factor)

You may want to use the exposedContentRect() here to make the behavior more predictable. Otherwise the behavior would change depending if the bars are in or out.

It probably does not matter much.

> Source/WebKit2/WebProcess/WebPage/ios/WebPageIOS.mm:1258
> -            newRange = Range::create(newRange->startContainer()->document(), newRange->startPosition(), currentRange->endPosition());
> +            newRange = Range::create(newRange->startContainer()->document(), newRange->endPosition(), currentRange->endPosition());
>          else
> -            newRange = Range::create(newRange->startContainer()->document(), currentRange->startPosition(), newRange->endPosition());
> +            newRange = Range::create(newRange->startContainer()->document(), currentRange->startPosition(), newRange->startPosition());

Can you add an explanation for this fix in the ChangeLog?
Comment 3 Enrica Casucci 2014-06-30 17:28:05 PDT
Committed revision 170620.