Bug 48545 - Home/End, PgUp, PgDwn, spacebar, shift+spacebar, etc. should respect writing-mode
Summary: Home/End, PgUp, PgDwn, spacebar, shift+spacebar, etc. should respect writing-...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Macintosh OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords: InRadar
Depends on:
Blocks: 46123
  Show dependency treegraph
 
Reported: 2010-10-28 11:56 PDT by Dave Hyatt
Modified: 2010-12-13 13:51 PST (History)
4 users (show)

See Also:


Attachments
Part 1: Patch for Mac WebKit 1 views. (6.23 KB, patch)
2010-12-09 12:10 PST, Dave Hyatt
no flags Details | Formatted Diff | Diff
Part 1: Patch for Mac WebKit 1 views. (4.64 KB, patch)
2010-12-09 12:28 PST, Dave Hyatt
no flags Details | Formatted Diff | Diff
Part 1: Patch for Mac WebKit 1 views. (4.67 KB, patch)
2010-12-09 13:05 PST, Dave Hyatt
mitz: review+
Details | Formatted Diff | Diff
Part 2: Patch for WebKit2 views. (26.88 KB, patch)
2010-12-13 11:55 PST, Dave Hyatt
sam: review+
Details | Formatted Diff | Diff
Cleanup on Windows. (2.66 KB, patch)
2010-12-13 12:28 PST, Dave Hyatt
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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