Bug 48545

Summary: Home/End, PgUp, PgDwn, spacebar, shift+spacebar, etc. should respect writing-mode
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: Layout and RenderingAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, mitz, webkit.review.bot
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 46123    
Attachments:
Description Flags
Part 1: Patch for Mac WebKit 1 views.
none
Part 1: Patch for Mac WebKit 1 views.
none
Part 1: Patch for Mac WebKit 1 views.
mitz: review+
Part 2: Patch for WebKit2 views.
sam: review+
Cleanup on Windows. aroben: review+

Description Dave Hyatt 2010-10-28 11:56:27 PDT
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.
Comment 1 Adele Peterson 2010-10-29 15:11:33 PDT
<rdar://problem/8612028>
Comment 2 Dave Hyatt 2010-12-09 12:10:42 PST
Created attachment 76101 [details]
Part 1: Patch for Mac WebKit 1 views.
Comment 3 Dave Hyatt 2010-12-09 12:28:01 PST
Created attachment 76110 [details]
Part 1: Patch for Mac WebKit 1 views.
Comment 4 mitz 2010-12-09 13:02:54 PST
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
Comment 5 Dave Hyatt 2010-12-09 13:05:07 PST
(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!
Comment 6 Dave Hyatt 2010-12-09 13:05:35 PST
Created attachment 76115 [details]
Part 1: Patch for Mac WebKit 1 views.
Comment 7 mitz 2010-12-09 13:41:15 PST
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
Comment 8 Darin Adler 2010-12-09 14:10:52 PST
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!
Comment 9 Dave Hyatt 2010-12-13 11:55:26 PST
Created attachment 76418 [details]
Part 2: Patch for WebKit2 views.
Comment 10 Sam Weinig 2010-12-13 11:59:41 PST
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?
Comment 11 Dave Hyatt 2010-12-13 12:14:47 PST
Fixed in r73941.
Comment 12 Dave Hyatt 2010-12-13 12:28:29 PST
Created attachment 76420 [details]
Cleanup on Windows.
Comment 13 Adam Roben (:aroben) 2010-12-13 12:29:10 PST
Comment on attachment 76420 [details]
Cleanup on Windows.

Is it possible to add a test for this?
Comment 14 WebKit Review Bot 2010-12-13 13:51:44 PST
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