Attachment #3011 for bug #4061

View Details Raw Unified Return to Bug 4061

| Differences between



2005-07-18  Trey Matteson  <trey@usa.net>

        Reviewed by John Sullivan.

        Fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4049
          scroll position not restored when going back/forward at ebay
        Fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4051
          Page jumps after loading even if I've scrolled while loading

        The short story is that attempting to restore the scroll position
        at the time when the first layout finishes addresses both of these
        issues.  An explanation of the underlying race condition is in
        a large comment near -_restoreScrollPosition.

        * WebCoreSupport.subproj/WebBridge.m:
        (-[WebBridge didFirstLayout]):  Pass through to WebFrame.
        * WebView.subproj/WebFrame.m:
        (-[WebFrame _transitionToCommitted:]):  Get rid of attempt to
          restoreScrollPosition that never did anything because the
          docView was always 0x0 size at that point.
        (-[WebFrame _opened]):  Get rid of redundant call to restoreScrollPosition.
          The imminent call to layoutCompleted makes the same call.
        (-[WebFrame _didFirstLayout]):  Restore the scroll position on
          first layout, if we're doing a b/f nav.
        * WebView.subproj/WebFrameInternal.h:

2005-07-18  John Sullivan  <sullivan@apple.com>

        Reviewed by Darin Adler.


        * WebView.subproj/WebUIDelegate.h: Fixed incorrect comment.

>>>>>>> 1.3131
2005-05-01  Darin Adler  <darin@apple.com>

        - move to Xcode native targets and stop checking in generated files

Lines 1509-1514 static id <WebFormDelegate> formDelegate ./WebCoreSupport.subproj/WebBridge.m_sec1
- ./WebCoreSupport.subproj/WebBridge.m +1 lines
1509		1509
1510	- (void)didFirstLayout	1510	- (void)didFirstLayout
1511	{	1511	{
		1512	[_frame _didFirstLayout];
1512	WebView *wv = [_frame webView];	1513	WebView *wv = [_frame webView];
1513	[[wv _frameLoadDelegateForwarder] webView:wv didFirstLayoutInFrame:_frame];	1514	[[wv _frameLoadDelegateForwarder] webView:wv didFirstLayoutInFrame:_frame];
1514	}	1515	}




- (void)_saveScrollPositionToItem:(WebHistoryItem *)item;
- (void)_restoreScrollPosition;
- (void)_scrollToTop;

- (WebHistoryItem *)_createItem: (BOOL)useOriginal;
- (WebHistoryItem *)_createItemTreeWithTargetFrame:(WebFrame *)targetFrame clippedAtTarget:(BOOL)doClip;

                    }
                    else
                        [self _makeDocumentView];
                        
                    // FIXME - I'm not sure this call does anything.  Should be dealt with as
                    // part of 3024377
                    [self _restoreScrollPosition];
                }
                break;


        }
        [view setNeedsLayout: YES];
        [view layout];

        
        NSArray *responses = [[self dataSource] _responses];
        NSURLResponse *response;
        int i, count = [responses count];

    }
}

/*
    There is a race condition between the layout and load completion that affects restoring the scroll position.
    We try to restore the scroll position at both the first layout and upon load completion.

    1) If first layout happens before the load completes, we want to restore the scroll position then so that the
       first time we draw the page is already scrolled to the right place, instead of starting at the top and later
       jumping down.  It is possible that the old scroll position is past the part of the doc laid out so far, in
       which case the restore silent fails and we will fix it in when we try to restore on doc completion.
    2) If the layout happens after the load completes, the attempt to restore at load completion time silently
       fails.  We then successfully restore it when the layout happens.
 */

- (void)_restoreScrollPosition
{
    ASSERT([_private currentItem]);

{
    [[self _bridge] unmarkAllMisspellings];
    [_private->children makeObjectsPerformSelector:@selector(_unmarkAllMisspellings)];
}

- (void)_didFirstLayout
{
    if ([[self webView] backForwardList]) {
        WebFrameLoadType loadType = [self _loadType];
        if (loadType == WebFrameLoadTypeForward ||
            loadType == WebFrameLoadTypeBack ||
            loadType == WebFrameLoadTypeIndexedBackForward)
        {
            [self _restoreScrollPosition];
        }
    }
}

@end

Lines 36-41 ./WebView.subproj/WebFrameInternal.h_sec1
- ./WebView.subproj/WebFrameInternal.h +1 lines
36	- (void)_setInternalLoadDelegate:(id)internalLoadDelegate;	36	- (void)_setInternalLoadDelegate:(id)internalLoadDelegate;
37	- (id)_internalLoadDelegate;	37	- (id)_internalLoadDelegate;
38	- (void)_unmarkAllMisspellings;	38	- (void)_unmarkAllMisspellings;
		39	- (void)_didFirstLayout;
39		40
40	- (NSURLRequest )_requestFromDelegateForRequest:(NSURLRequest )request identifier:(id )identifier error:(NSError *)error;	41	- (NSURLRequest )_requestFromDelegateForRequest:(NSURLRequest )request identifier:(id )identifier error:(NSError *)error;
41	- (void)_sendRemainingDelegateMessagesWithIdentifier:(id)identifier response:(NSURLResponse )response length:(unsigned)length error:(NSError )error;	42	- (void)_sendRemainingDelegateMessagesWithIdentifier:(id)identifier response:(NSURLResponse )response length:(unsigned)length error:(NSError )error;

Return to Bug 4061

Lines 178-184 NSString *WebPageCacheDocumentViewKey = ./WebView.subproj/WebFrame.m_sec1
- ./WebView.subproj/WebFrame.m -7 / +26 lines
178		178
179	- (void)_saveScrollPositionToItem:(WebHistoryItem *)item;	179	- (void)_saveScrollPositionToItem:(WebHistoryItem *)item;
180	- (void)_restoreScrollPosition;	180	- (void)_restoreScrollPosition;
181	- (void)_scrollToTop;
182		181
183	- (WebHistoryItem *)_createItem: (BOOL)useOriginal;	182	- (WebHistoryItem *)_createItem: (BOOL)useOriginal;
184	- (WebHistoryItem )_createItemTreeWithTargetFrame:(WebFrame )targetFrame clippedAtTarget:(BOOL)doClip;	183	- (WebHistoryItem )_createItemTreeWithTargetFrame:(WebFrame )targetFrame clippedAtTarget:(BOOL)doClip;
Lines 794-803 NSString *WebPageCacheDocumentViewKey = ./WebView.subproj/WebFrame.m_sec2
794	}	793	}
795	else	794	else
796	[self _makeDocumentView];	795	[self _makeDocumentView];
797
798	// FIXME - I'm not sure this call does anything. Should be dealt with as
799	// part of 3024377
800	[self _restoreScrollPosition];
801	}	796	}
802	break;	797	break;
803		798
Lines 1069-1076 static CFAbsoluteTime _timeOfLastComplet ./WebView.subproj/WebFrame.m_sec3
1069	}	1064	}
1070	[view setNeedsLayout: YES];	1065	[view setNeedsLayout: YES];
1071	[view layout];	1066	[view layout];
1072	[self _restoreScrollPosition];	1067
1073
1074	NSArray *responses = [[self dataSource] _responses];	1068	NSArray *responses = [[self dataSource] _responses];
1075	NSURLResponse *response;	1069	NSURLResponse *response;
1076	int i, count = [responses count];	1070	int i, count = [responses count];
Lines 2093-2098 static CFAbsoluteTime _timeOfLastComplet ./WebView.subproj/WebFrame.m_sec4
2093	}	2087	}
2094	}	2088	}
2095		2089
		2090	/*
		2091	There is a race condition between the layout and load completion that affects restoring the scroll position.
		2092	We try to restore the scroll position at both the first layout and upon load completion.
		2093
		2094	1) If first layout happens before the load completes, we want to restore the scroll position then so that the
		2095	first time we draw the page is already scrolled to the right place, instead of starting at the top and later
		2096	jumping down. It is possible that the old scroll position is past the part of the doc laid out so far, in
		2097	which case the restore silent fails and we will fix it in when we try to restore on doc completion.
		2098	2) If the layout happens after the load completes, the attempt to restore at load completion time silently
		2099	fails. We then successfully restore it when the layout happens.
		2100	*/
		2101
2096	- (void)_restoreScrollPosition	2102	- (void)_restoreScrollPosition
2097	{	2103	{
2098	ASSERT([_private currentItem]);	2104	ASSERT([_private currentItem]);
Lines 2700-2705 static CFAbsoluteTime _timeOfLastComplet ./WebView.subproj/WebFrame.m_sec5
2700	{	2706	{
2701	[[self _bridge] unmarkAllMisspellings];	2707	[[self _bridge] unmarkAllMisspellings];
2702	[_private->children makeObjectsPerformSelector:@selector(_unmarkAllMisspellings)];	2708	[_private->children makeObjectsPerformSelector:@selector(_unmarkAllMisspellings)];
		2709	}
		2710
		2711	- (void)_didFirstLayout
		2712	{
		2713	if ([[self webView] backForwardList]) {
		2714	WebFrameLoadType loadType = [self _loadType];
		2715	if (loadType == WebFrameLoadTypeForward \|\|
		2716	loadType == WebFrameLoadTypeBack \|\|
		2717	loadType == WebFrameLoadTypeIndexedBackForward)
		2718	{
		2719	[self _restoreScrollPosition];
		2720	}
		2721	}
2703	}	2722	}
2704		2723
2705	@end	2724	@end

Lines 1-3 ChangeLog_sec1
- ChangeLog -1 / +26 lines
		1	2005-07-18 Trey Matteson <trey@usa.net>
		2
		3	Reviewed by John Sullivan.
		4
		5	Fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4049
		6	scroll position not restored when going back/forward at ebay
		7	Fixed http://bugzilla.opendarwin.org/show_bug.cgi?id=4051
		8	Page jumps after loading even if I've scrolled while loading
		9
		10	The short story is that attempting to restore the scroll position
		11	at the time when the first layout finishes addresses both of these
		12	issues. An explanation of the underlying race condition is in
		13	a large comment near -_restoreScrollPosition.
		14
		15	* WebCoreSupport.subproj/WebBridge.m:
		16	(-[WebBridge didFirstLayout]): Pass through to WebFrame.
		17	* WebView.subproj/WebFrame.m:
		18	(-[WebFrame _transitionToCommitted:]): Get rid of attempt to
		19	restoreScrollPosition that never did anything because the
		20	docView was always 0x0 size at that point.
		21	(-[WebFrame _opened]): Get rid of redundant call to restoreScrollPosition.
		22	The imminent call to layoutCompleted makes the same call.
		23	(-[WebFrame _didFirstLayout]): Restore the scroll position on
		24	first layout, if we're doing a b/f nav.
		25	* WebView.subproj/WebFrameInternal.h:
		26
1	2005-07-18 John Sullivan <sullivan@apple.com>	27	2005-07-18 John Sullivan <sullivan@apple.com>
2		28
3	Reviewed by Darin Adler.	29	Reviewed by Darin Adler.
Lines 1356-1362 ChangeLog_sec2
1356		1382
1357	* WebView.subproj/WebUIDelegate.h: Fixed incorrect comment.	1383	* WebView.subproj/WebUIDelegate.h: Fixed incorrect comment.
1358		1384
1359	>>>>>>> 1.3131
1360	2005-05-01 Darin Adler <darin@apple.com>	1385	2005-05-01 Darin Adler <darin@apple.com>
1361		1386
1362	- move to Xcode native targets and stop checking in generated files	1387	- move to Xcode native targets and stop checking in generated files