[Webkit-unassigned] [Bug 122062] Web Inspector: content view in back/forward list multiple times won't restore earlier positions

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=122062





--- Comment #4 from Timothy Hatcher <timothy at apple.com>  2013-09-28 08:35:26 PST ---
(From update of attachment 212900)
View in context: https://bugs.webkit.org/attachment.cgi?id=212900&action=review

> Source/WebInspectorUI/UserInterface/ContentViewContainer.js:152
> +        if (currentEntry && currentEntry.contentView === contentView && Object.shallowEqual(currentEntry.cookie, newEntry.cookie))

Object.shallowEqual will not compare the positionToReveal or textRangeToSelect objects. So a navigation to the same location will push another entry. Perhaps we need a more advanced Object.shallowEqual that does an a.equals(b) if the objects are the same type/constructor and has an equals function.

> Source/WebInspectorUI/UserInterface/ContentViewContainer.js:440
> +        entry.restorePositions();
>          this._restoreScrollPositionsForContentView(contentView);

Should these lines flip, or do they not interfere in practice?

> Source/WebInspectorUI/UserInterface/ResourceSidebarPanel.js:266
> +        var cookie = {};
> +        cookie.positionToReveal = positionToReveal;
> +        cookie.textRangeToSelect = textRangeToSelect;
> +        cookie.forceUnformatted = forceUnformatted;

This could be one line. I'm not sure textRangeToSelect should be in the cookie. That doesn't get restored in Xcode when going back/forward. Also forceUnformatted seems weird to trigger. I would only expect position.

Also the position will highlight/flash when revealed. That might be weird on back/forward vs a direct navigation.

> Source/WebInspectorUI/UserInterface/ResourceSidebarPanel.js:267
> +        var restoreCallback = function(contentView, cookie) {

I'd put a newline before this line to separate out the function so it isn't as hidden in a large block.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.