766937 - Add scrollTopMax/scrollLeftMax properties to elements

Status: RESOLVED FIXED

(Core :: DOM: CSS Object Model, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

See https://bugzilla.mozilla.org/show_bug.cgi?id=755971#c75 for context. This bug is tracking the addition of scrollMaxX and scrollMaxY properties on elements, so that content can determine how much the element is capable of scrolling. These should always return the largest values scrollLeft/scrollTop will return, and are more or less equal to element.scroll(Width|Height) - element.client(Width|Height), modulo rounding errors.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

This needs to be discussed on www-style before it's landed.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Add scrollMaxX and scrollMaxY to elements

Verified that this returns the correct value for the d1 div in the test case at https://bugzilla.mozilla.org/attachment.cgi?id=635179&action=edit

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 635352 [details] [diff] [review]
Add scrollMaxX and scrollMaxY to elements

Review of attachment 635352 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/base/src/nsGenericElement.cpp
@@ +2201,5 @@
> +nsGenericElement::GetScrollMaxX()
> +{
> +  if (IsSVG()) {
> +    return 0;
> +  }

This IsSVG check seems redundant. Presumably GetScrollFrame() will return null in that case.

@@ +2208,5 @@
> +  if (!sf) {
> +    return 0;
> +  }
> +
> +  return nsPresContext::AppUnitsToIntCSSPixels(sf->GetScrollRange().width);

Actually you want XMost(), for now.

::: dom/interfaces/core/nsIDOMElement.idl
@@ +156,5 @@
>     */
>    readonly attribute long             clientHeight;
>  
> +  /* The maximum offset that the element can be scrolled to
> +     (i.e., the element scrollWidth/height minus the element clientWidth/height) */

I would say "the value that scrollLeft/scrollTop would be clamped to if they were set to arbitrary large values".

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to :Ms2ger from comment #1)
> This needs to be discussed on www-style before it's landed.

Yes, I'll do that. Although I doubt there will be useful feedback.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Add scrollMaxX and scrollMaxY to elements (v2)

Good catches, thanks. Updated to address all the comments. I also have a simple test page at http://people.mozilla.org/~kgupta/bug/766937.html that tests both the LTR and RTL cases to verify the XMost() thing is working as expected.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 635732 [details] [diff] [review]
Add scrollMaxX and scrollMaxY to elements (v2)

Review of attachment 635732 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, but on reflection I think scrollLeftMax and scrollTopMax are better names.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Attachment: Add scrollLeftMax and scrollTopMax to elements (v3)

Updated to be scrollLeftMax and scrollTopMax as requested.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Should I land this patch, or do you want to wait longer for feedback on www-style?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You should land it. There's no feedback on www-style yet and I don't expect to get any.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/01c8479cf333

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/01c8479cf333

Comment 12

[Jean-Yves Perrier [:teoli]]

At least documented:
https://developer.mozilla.org/en-US/docs/Web/API/Element
https://developer.mozilla.org/en-US/docs/Web/API/Element.scrollTopMax
https://developer.mozilla.org/en-US/docs/Web/API/Element.scrollLeftMax
https://developer.mozilla.org/en-US/Firefox/Releases/16

(Note that, as far I know, this is not part of any specification.