Bug 1558616 Comment 3 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

To restate investigation from IRC, it turns out we have potentially racing logic:
- dxr.js' `scrollIntoView` is invoked by dxr.js' `$(function() {...})` which also happens at document ready time.
- code-highlighter.js runs manual `scrollTo` calls at `$(document).ready()` time.

The script tags are in the order "dxr.js" followed by "code-highlighter.js" so it's likely the above ordering is also the ordering in which the code runs.

The scrollIntoView implementation is basically trying to get the browser to do its own native scrolling to an anchor position.  If an element exists with the given id already, it just leaves it up to the browser.  If the element doesn't exist, it creates a div with the given id and inserts it into the DOM and hopes that the browser will then automatically scroll to that.  If the browser isn't Firefox, the logic redundantly writes to window.location to trigger Chrome's native anchor scrolling.

Because the actual line-number id's are of the form `l${lineNum}`, we do expect the scrollIntoView logic to create a div with id `${lineNum}` the first time the event fires for the line number.  So for single-line highlighting, it is doing something.  We also expect this to end up being somewhat of a no-op during interactive use where the user clicks around because the browser should see that the line number is already visible.

It seems like the logic running concurrently at startup breaks things, presumably due to APZ and stale offset values interfering with what code-highlighter.js is trying to do manually with scroll offsets.

The simple quick hack we just tried on IRC was to make scrollIntoView() simply immediately return.  This stopped it from interfering with the sufficiently correct code-highlighter.js ready logic.

dxr.js hooks up scrollIntoView to window.onhashchange and so I think the main question is whether the hashchange logic is doing something helpful when navigating within the file via "go to definition" or using the browser's forward/back buttons (within a file).
To restate investigation from IRC, it turns out we have potentially racing logic:
- dxr.js' `scrollIntoView` is invoked by dxr.js' `$(function() {...})` which happens at document ready time.
- code-highlighter.js runs manual `scrollTo` calls at `$(document).ready()` time too.

The script tags are in the order "dxr.js" followed by "code-highlighter.js" so it's likely the above ordering is also the ordering in which the code runs.

The scrollIntoView implementation is basically trying to get the browser to do its own native scrolling to an anchor position.  If an element exists with the given id already, it just leaves it up to the browser.  If the element doesn't exist, it creates a div with the given id and inserts it into the DOM and hopes that the browser will then automatically scroll to that.  If the browser isn't Firefox, the logic redundantly writes to window.location to trigger Chrome's native anchor scrolling.

Because the actual line-number id's are of the form `l${lineNum}`, we do expect the scrollIntoView logic to create a div with id `${lineNum}` the first time the event fires for the line number.  So for single-line highlighting, it is doing something.  We also expect this to end up being somewhat of a no-op during interactive use where the user clicks around because the browser should see that the line number is already visible.

It seems like the logic running concurrently at startup breaks things, presumably due to APZ and stale offset values interfering with what code-highlighter.js is trying to do manually with scroll offsets.

The simple quick hack we just tried on IRC was to make scrollIntoView() simply immediately return.  This stopped it from interfering with the sufficiently correct code-highlighter.js ready logic.

dxr.js hooks up scrollIntoView to window.onhashchange and so I think the main question is whether the hashchange logic is doing something helpful when navigating within the file via "go to definition" or using the browser's forward/back buttons (within a file).
To restate investigation from IRC, it turns out we have potentially racing logic:
- dxr.js' `scrollIntoView` is invoked by dxr.js' `$(function() {...})` which happens at document ready time.  https://github.com/mozsearch/mozsearch/blob/master/static/js/dxr.js#L59
- code-highlighter.js runs manual `scrollTo` calls at `$(document).ready()` time too.   https://github.com/mozsearch/mozsearch/blob/master/static/js/code-highlighter.js#L496

The script tags are in the order "dxr.js" followed by "code-highlighter.js" so it's likely the above ordering is also the ordering in which the code runs.

The scrollIntoView implementation is basically trying to get the browser to do its own native scrolling to an anchor position.  If an element exists with the given id already, it just leaves it up to the browser.  If the element doesn't exist, it creates a div with the given id and inserts it into the DOM and hopes that the browser will then automatically scroll to that.  If the browser isn't Firefox, the logic redundantly writes to window.location to trigger Chrome's native anchor scrolling.

Because the actual line-number id's are of the form `l${lineNum}`, we do expect the scrollIntoView logic to create a div with id `${lineNum}` the first time the event fires for the line number.  So for single-line highlighting, it is doing something.  We also expect this to end up being somewhat of a no-op during interactive use where the user clicks around because the browser should see that the line number is already visible.

It seems like the logic running concurrently at startup breaks things, presumably due to APZ and stale offset values interfering with what code-highlighter.js is trying to do manually with scroll offsets.

The simple quick hack we just tried on IRC was to make scrollIntoView() simply immediately return.  This stopped it from interfering with the sufficiently correct code-highlighter.js ready logic.

dxr.js hooks up scrollIntoView to window.onhashchange and so I think the main question is whether the hashchange logic is doing something helpful when navigating within the file via "go to definition" or using the browser's forward/back buttons (within a file).

Back to Bug 1558616 Comment 3