bdkjones / CodeKit-1-Old

147 stars 1 forks source link

TypeError: Cannot set property 'scrollTop' of null #666

Open markjaquith opened 10 years ago

markjaquith commented 10 years ago

On some pages, the auto reload feature seems to be causing the following JavaScript error:

TypeError: Cannot set property 'scrollTop' of null

Apologies if this is a goose chase, but this does seem to be the culprit, as normal reloads don't cause this JS issue and the the JS error has no filename (which suggests it's not anything on the page).

An example URL where it happens is http://www.experts-exchange.com/

Reproduction:

  1. Go to http://www.experts-exchange.com/ and leave it as the active tab
  2. Fire up Code Kit
  3. Save a project file

There should be a JavaScript error in the console, but not corresponding to any file or any line.

Is CodeKit attempting to inject JavaScript somehow? Is it doing something with scrollTop? If so, could it make sure the thing it's calling foo.scrollTop = bar on make sure foo isn't null first?

markjaquith commented 10 years ago

$ ack -a scrollTop /Applications/CodeKit.app/Contents/MacOS/CodeKit Binary file /Applications/CodeKit.app/Contents/MacOS/CodeKit matches

Might not be crazy, then.

bdkjones commented 10 years ago

Hi Mark,

Yep. When CodeKit reloads an entire page (rather than injecting new CSS into the existing DOM) it records the previous scroll position so that it can restore that position when the new DOM is loaded. Otherwise, full-page reloads would snap you back to the top of the page each time they occurred.

This may be a timing issue, as CodeKit simply waits a hard-coded amount of time before telling the browser to restore the scroll position. This delay allows the new DOM to load before the Javascript that restores that position is fired. If experts-exchange takes longer than the delay to reload the page, then the Javascript call to restore scroll position is fired too early and you would see an issue like this.

Fortunately, it won't be a problem in 2.0!

markjaquith commented 10 years ago

This may be a timing issue

That matches my observations. Some sites do it every time. Others never. Others sometimes.

What's the timeline on 2.0? Is a 1.8.x update a possibility to at least do a null check before setting scrollTop? I mean, the scroll position restoration isn't working anyway in this case, and the JavaScript error means that when I'm doing JS development I have to keep checking the console to see if a JS error is a real error that I need to fix or just a CodeKit artifact.