Bug 48545 – Home/End, PgUp, PgDwn, spacebar, shift+spacebar, etc. should respect writing-mode

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Dave Hyatt]

The arrow keys, spacebar, shift+spacebar, etc. should respect writing-mode and move in the appropriate writing-mode direction rather than being hardcoded to horizontal-tb.

[Adele Peterson]

<rdar://problem/8612028>

[Dave Hyatt]

Created attachment 76101 [details]
Part 1: Patch for Mac WebKit 1 views.

[Dave Hyatt]

Created attachment 76110 [details]
Part 1: Patch for Mac WebKit 1 views.

Comment on attachment 76110 [details]
Part 1: Patch for Mac WebKit 1 views.

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

> WebKit/mac/WebView/WebFrameView.mm:595
> +    if (isVertical) {
> +        if (!isFlipped)
> +            NSMakePoint(frame.origin.x, NSMaxY(frame));
> +        else
> +            NSMakePoint(frame.origin.x, NSMinY(frame));
> +    } else {
> +        if (!isFlipped)
> +            NSMakePoint(NSMaxX(frame), frame.origin.y);
> +        else
> +            NSMakePoint(NSMinX(frame), frame.origin.y);
> +    }

I think you meant to assign the result of NSMakePoint() to point

[Dave Hyatt]

(In reply to comment #4)
> (From update of attachment 76110 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76110&action=review
> 
> > WebKit/mac/WebView/WebFrameView.mm:595
> > +    if (isVertical) {
> > +        if (!isFlipped)
> > +            NSMakePoint(frame.origin.x, NSMaxY(frame));
> > +        else
> > +            NSMakePoint(frame.origin.x, NSMinY(frame));
> > +    } else {
> > +        if (!isFlipped)
> > +            NSMakePoint(NSMaxX(frame), frame.origin.y);
> > +        else
> > +            NSMakePoint(NSMinX(frame), frame.origin.y);
> > +    }
> 
> I think you meant to assign the result of NSMakePoint() to point

WORST PATCH EVER!

[Dave Hyatt]

Created attachment 76115 [details]
Part 1: Patch for Mac WebKit 1 views.

Comment on attachment 76115 [details]
Part 1: Patch for Mac WebKit 1 views.

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

> WebKit/mac/WebView/WebFrameView.mm:604
> +    

greenspace

[Darin Adler]

Comment on attachment 76115 [details]
Part 1: Patch for Mac WebKit 1 views.

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

> WebKit/mac/WebView/WebFrameView.mm:544
> +    return renderView->style()->isHorizontalWritingMode();

Sense here seems backwards!

[Dave Hyatt]

Created attachment 76418 [details]
Part 2: Patch for WebKit2 views.

[Sam Weinig]

Comment on attachment 76418 [details]
Part 2: Patch for WebKit2 views.

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

> WebKit2/WebProcess/WebPage/mac/WebPageMac.mm:112
> +static inline void logicalScroll(Page* page, ScrollLogicalDirection direction, ScrollGranularity granularity)
> +{
> +    page->focusController()->focusedOrMainFrame()->eventHandler()->logicalScrollRecursively(direction, granularity);
> +}

Is anyone still calling the scroll() function above? Can we remove it?

[Dave Hyatt]

Fixed in r73941.

[Dave Hyatt]

Created attachment 76420 [details]
Cleanup on Windows.

[Adam Roben (:aroben)]

Comment on attachment 76420 [details]
Cleanup on Windows.

Is it possible to add a test for this?

[WebKit Review Bot]

http://trac.webkit.org/changeset/73941 might have broken SnowLeopard Intel Release (Tests) and GTK Linux 64-bit Debug
The following tests are not passing:
editing/selection/extend-to-line-boundary.html