The default bug view has changed. See this FAQ.

Add scrollTopMax/scrollLeftMax properties to elements

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: CSS Object Model
RESOLVED FIXED
5 years ago
3 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

({dev-doc-complete})

Trunk
mozilla16
dev-doc-complete
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
This needs to be discussed on www-style before it's landed.
Keywords: dev-doc-needed
Created attachment 635352 [details] [diff] [review]
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
Attachment #635352 - Flags: review?(roc)
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".
(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.
Created attachment 635732 [details] [diff] [review]
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.
Attachment #635352 - Attachment is obsolete: true
Attachment #635352 - Flags: review?(roc)
Attachment #635732 - Flags: review?(roc)
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.
Created attachment 636415 [details] [diff] [review]
Add scrollLeftMax and scrollTopMax to elements (v3)

Updated to be scrollLeftMax and scrollTopMax as requested.
Attachment #635732 - Attachment is obsolete: true
Attachment #635732 - Flags: review?(roc)
Attachment #636415 - Flags: review?(roc)
Attachment #636415 - Flags: review?(roc) → review+
Should I land this patch, or do you want to wait longer for feedback on www-style?
You should land it. There's no feedback on www-style yet and I don't expect to get any.
Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/01c8479cf333
https://hg.mozilla.org/mozilla-central/rev/01c8479cf333
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Depends on: 962249
Summary: Add scrollMaxX/Y properties to elements → Add scrollTopMax/scrollLeftMax properties to elements
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.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.