Bug 96539

Summary:

REGRESSION(r106730): iframes with scrolling=no can't scroll to anchors

Product:

WebKit

Reporter:

Nate Chapin <japhet>

Component:

Frames

Assignee:

Nate Chapin <japhet>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, simon.fraser, spottabathini, tonikitoo, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

61558

Bug Blocks:

Attachments:

Description	Flags
patch	none
Patch for landing	none

Description Nate Chapin 2012-09-12 12:08:13 PDT

Original report: http://code.google.com/p/chromium/issues/detail?id=119666

If an iframe with scrolling=no contains a link to an anchor in the same document, we do not properly scroll to the anchor when it is clicked.

This appears to be a regression from http://trac.webkit.org/changeset/106730 .

Comment 1 Nate Chapin 2012-09-12 12:18:54 PDT

Created attachment 163670 [details]
patch

Comment 2 Antonio Gomes 2012-10-02 12:44:20 PDT

Comment on attachment 163670 [details]
patch

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

> Source/WebCore/rendering/RenderLayer.cpp:1793
> +                if ((frameElement && frameElement->scrollingMode() != ScrollbarAlwaysOff) || (!frameView->frame()->eventHandler()->autoscrollInProgress() && !frameView->wasScrolledByUser())) {

it is turning to be a long line.

Comment 3 Nate Chapin 2012-10-02 15:42:15 PDT

Created attachment 166765 [details]
Patch for landing

Comment 4 WebKit Review Bot 2012-10-02 16:14:49 PDT

Comment on attachment 166765 [details]
Patch for landing

Clearing flags on attachment: 166765

Committed r130226: <http://trac.webkit.org/changeset/130226>

Comment 5 WebKit Review Bot 2012-10-02 16:14:52 PDT

All reviewed patches have been landed.  Closing bug.

Comment 6 Darin Adler 2012-10-02 16:45:34 PDT

Comment on attachment 163670 [details]
patch

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

>> Source/WebCore/rendering/RenderLayer.cpp:1793
>> +                if ((frameElement && frameElement->scrollingMode() != ScrollbarAlwaysOff) || (!frameView->frame()->eventHandler()->autoscrollInProgress() && !frameView->wasScrolledByUser())) {
> 
> it is turning to be a long line.

An inline helper function could spread this out over multiple lines and allow a comment on each line.

Comment 7 Antonio Gomes 2012-10-02 19:43:24 PDT

> > 
> > it is turning to be a long line.
> 
> An inline helper function could spread this out over multiple lines and allow a comment on each line.

Nate, lets please go for it.