Bug 57423 – REGRESSION (r82320): Spacebar no longer pages down

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Kevin M. Dean]

Pressing the spacebar no longer scrolls the page down on any site.

[Alexey Proskuryakov]

This must be my fault.

[Alexey Proskuryakov]

Created attachment 87571 [details]
proposed fix

[Alexey Proskuryakov]

Created attachment 87574 [details]
updated patch

I decided to make some more changes for consistency at the risk of breaking stuff.

[Alexey Proskuryakov]

<rdar://problem/9207702>

[Darin Adler]

Comment on attachment 87574 [details]
updated patch

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

> Source/WebKit/mac/ChangeLog:15
> +        Even if this specific command hasn't been handled, that doens't nullify side effects from

Typo in the word doesn't

> Source/WebKit/mac/ChangeLog:20
> +        bug, removed setting _private->interpretKeyEventsParameters to 0. I don't see any way for
> +        another WebHTMLView NSTextInput method to be called from within insertText:, so no one is
> +        going to look at it.

Seems OK, but risky.

> Source/WebKit/mac/WebView/WebHTMLView.mm:5946
> -        NSString *rangeString = [string attribute:NSTextInputReplacementRangeAttributeName atIndex:0 longestEffectiveRange:NULL inRange:NSMakeRange(0, markedTextLength)];
> +        NSString *rangeString = [string attribute:NSTextInputReplacementRangeAttributeName atIndex:0 longestEffectiveRange:nil inRange:NSMakeRange(0, markedTextLength)];

This is a pointer to an NSRange, not a pointer to an Objective-C object, so the use of nil is not appropriate.

> Source/WebKit/mac/WebView/WebHTMLView.mm:6033
> -        // event in TSM.  This behaviour matches that of -[WebHTMLView setMarkedText:selectedRange:] when it receives an
> +        // event in TSM. This behaviour matches that of -[WebHTMLView setMarkedText:selectedRange:] when it receives an

If you’re tweaking things, you could move to the U.S. spelling of behavior.

> Source/WebKit/mac/WebView/WebHTMLView.mm:6073
> +    bool eventHandled = false;
>      String eventText = text;
>      eventText.replace(NSBackTabCharacter, NSTabCharacter); // same thing is done in KeyEventMac.mm in WebCore
>      if (coreFrame && coreFrame->editor()->canEdit()) {
> -        if (!coreFrame->editor()->hasComposition())
> -            coreFrame->editor()->insertText(eventText, event);
> -        else
> +        if (!coreFrame->editor()->hasComposition()) {
> +            // An insertText: might be handled by other responders in the chain if we don't handle it.
> +            // One example is space bar that results in scrolling down the page.
> +            eventHandled = coreFrame->editor()->insertText(eventText, event);
> +        } else {
> +            eventHandled = true;
>              coreFrame->editor()->confirmComposition(eventText);
> +        }
>      }
>      
>      if (parameters)
> -        parameters->eventInterpretationHadSideEffects = true;
> -
> -    _private->interpretKeyEventsParameters = parameters;
> +        parameters->eventInterpretationHadSideEffects |= eventHandled;

I don’t see why we are doing the work to make eventText and set up the eventHandled boolean in cases where we don’t need them. I would put more of the code inside the if, or use an early return instead of an if.

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/82497>.

[Alexey Proskuryakov]

*** Bug 57580 has been marked as a duplicate of this bug. ***

[Alexey Proskuryakov]

*** Bug 57555 has been marked as a duplicate of this bug. ***

[Leif Joel Örell]

Resolved fixed in Safari but not in Safari-Reader. Spacebar still no longer pages down in Safari-Reader

[Alexey Proskuryakov]

I cannot reproduce the problem with Safari Reader in r82773 nightly. I tried Reader on several articles from WebKit blog, including <http://www.webkit.org/blog/1580/user-agent-string-changes-on-webkit-trunk/>.

[Kevin M. Dean]

(In reply to comment #10)
> I cannot reproduce the problem with Safari Reader in r82773 nightly. I tried Reader on several articles from WebKit blog, including <http://www.webkit.org/blog/1580/user-agent-string-changes-on-webkit-trunk/>.

When I try the blog link with the same nightly under 10.5.8 PPC, Reader is really messed up. It loads scrolled to the bottom initally. I drag it back to the top, hit spacebar and it only advances about 10 pixels at a time. I press ESC to close Reader and if I click to reopen in it no longer appears at all even if I reload the page. I have to close the window completely and load the link into a new window for Reader even to appear again, but with the same issues. Something much more damaged than just the spacebar for me there. I'll check for an extenstion conflict just in case.

[Kevin M. Dean]

OK, no extension conflict and also works fine in Safari 5.0.4 with same extensions. The Reader sliding in is messed up as well. Normally it just slides up from the bottom and stops at the top but now it slides up to where the bottom of the reader page is practically at the top of my screen with a weird shaded line in the middle and then it snaps back. I have a feeling when I reopen the reader, it's not that it's not opening, it's opening somewhere off-screen and inaccessible, maybe?

I'll upload a quick screenshot.

[Kevin M. Dean]

Created attachment 87996 [details]
Reader initial open glitch

[Kevin M. Dean]

If I Disable Accelerated Compositing it works more like expected without the placement glitches but the spacebar still only moves about 10 pixels.

[Alexey Proskuryakov]

These sound like two bugs that should be reported separately.

[Kevin M. Dean]

(In reply to comment #15)
> These sound like two bugs that should be reported separately.

I'm in the process of tracking down when the problems occur in the nightlies. They go way back and appear at different times. Once I have it figured out, I'll open a new bug for Reader.

[Kevin M. Dean]

OK, new bug open at Bug 57720

So, the spacebar bug has been around in Reader since January 14th 2011.
...the initial open being glitchy since October 22nd 2010
...not being able to reopen Reader once it was opened and closed since September 16th 2010
...and Reader opening scrolled to the bottom and not the top since July 19th 2010.

So, it goes to show you that I never use Reader or else this would have been at least caught by me sooner.