Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #11306 (closed defect: fixed)

Opened 2 months ago

Last modified 1 week ago

[PATCH] Bug in Effect.ScrollTo

Reported by: staaky Assigned to: madrobby
Priority: normal Milestone: 2.x
Component: script.aculo.us Version: edge
Severity: normal Keywords:
Cc:

Description

I'm not sure why this bug sneaked in with patch #10245, but here's what's up:

http://dev.rubyonrails.org/attachment/ticket/10245/fix_firefox_scrolling_bug.diff

document.viewport.getScrollOffsets[0] ? that will always return undefined. This was mistaken for a fix since the undefined sort of fixes the problem, but it's not the expected result.

I guess what he was trying todo is: 

scrollOffsets = document.viewport.getScrollOffsets(),
elementOffsets = $(element).cumulativeOffset(),
max = scrollOffsets[0] - document.viewport.getHeight();

But that doesn't solve anything. Hmm.. I'll have to look at this a bit more to see what you are expecting to get from max before I patch this up.

Attachments

effect_scrollto_fix.diff (1.0 kB) - added by staaky on 03/09/08 15:21:20.

Change History

03/09/08 15:21:20 changed by staaky

  • attachment effect_scrollto_fix.diff added.

03/09/08 15:22:29 changed by staaky

I see now that max is the maximum amount of scroll height (-viewport height).

Since there's no good way to get the scroll height in Prototype at the moment let's just depend on scrollTo to do the right thing and not use max at all. Added patch to do this.

Here's also a quick attempt at scrollDimensions, still not cross-browser but might be a good start if ever needed in Prototype. 

scrollDimensions: function() {
  var dimensions = {};
  $w('width height').each(function(d) {
    var D = d.capitalize(), B = Prototype.Browser,
    ddE = document.documentElement;
    dimensions[d] = B.IE ? [ddE['offset' + D], ddE['scroll' + D]].max() :
      B.WebKit ? document.body['scroll' + D] : ddE['scroll' + D];
    });
    return dimensions;
},

03/09/08 15:34:27 changed by staaky

  • summary changed from Bug in Effect.ScrollTo to [PATCH] Bug in Effect.ScrollTo.

05/05/08 23:23:48 changed by staaky

  • status changed from new to closed.
  • resolution set to fixed.

Fixed in git