Closed Bug 111034 Opened 19 years ago Closed 12 years ago

XUL needs clientwidth/height or scrollheight/width properties

Categories

(Core :: XUL, enhancement)

x86
Other
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9.1a2

People

(Reporter: rvj, Assigned: enndeakin)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 4 obsolete files)

I understand via the DOM newgroup that  the current viewport properties have 
been written for html only.

I would argue that there is just as much need (if not more) to make this 
functionality available for XUL applications because document browsing is most 
likely to be controlled via the user agent (rather than the document)

As a minimum could there be support for the following?

window.frames[xulwindowname].document.scrollHeight
window.frames[xulwindowname].document.scrollWidth
xul
Assignee: joki → hyatt
Severity: normal → enhancement
Status: UNCONFIRMED → NEW
Component: DOM Events → XP Toolkit/Widgets: XUL
Ever confirmed: true
QA Contact: vladimire → jrgm
where xulwindowname just means any valid  reference to a xul 
iframe/browser/editor element
CC'ing jst.
1) Even HTML documents don't have scroll* properties available, only HTML
elements and windows.
2) window.frames[xulwindowname].scrollX and window.frames[xulwindowname].scrollY
probably do what you want.

Is it scroll* properties for XUL *elements* (like box, I assume?) you are
requesting?
I guess I am beginning to get a bit punch drunk with sytax. 

What ideally I want  to do is compare the window.innerWidth(innerHeight) with 
the window.scrollWidth(scrollHeight) and perhaps other properties to determine 
if a particular scrollbar is  displayed.

Since  window.scrollWidth is not a supported property  then I presume I need to 
fetch the scrollwidth(or clientWidth) of the document within it.?

Im not sure if this approach will work  but surely it ought to be possible to 
could return the document's width (not including  scrollbars)

PS

1. Im not sure what window.frames[xulwindowname].scrollX and window.frames
[xulwindowname].scrollY are supposed to do. Can you explain? Im aware of the 
now depreciated scroll method but that has been replaced by ScrollBy.


2. MORE importantly Im trying to find out the height of document for a given 
window. Now with htm documents this is simple.  e.g. window.frames
[x].document.height.

But with xml this is inappropriate - I believe you have to use computed style 
to get the current document height based on the root node.


Now what I had hoped is that a single method/property could be used to return 
the current document height irrespective of file type. Is there such a property 
or could scrollheight be used to claculate the correct depth


 



you can mark this bug invalid if you like 

Ive finally found a truely awful work around for detecting scrollbar 
visibility !!!
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.1
If you've got a way to scroll XUL elements, can you please post your solution
somewhere? 
This bug, as filed, probably is invalid.

I'm wondering, though... can there be some standard way to expose a box's 
scrollbars to the DOM, in all incarnations?  If there is a bug already filed 
for this, I'm not sure where.
This patch just moves the offsetXXX/clientXXX/scrollXXX properties into GenericElement. The offset/scroll/clientWidth/clientHeight properties are useful, but I can't figure out what clientLeft/clientTop are supposed to be values of.

Most of this patch is just moving code and the addition of two testcases. The only other code change of note is in GetOffsetRect so that popup frames are treated as offset parents.
Assignee: hyatt → enndeakin
Attachment #266392 - Flags: superreview?(roc)
Attachment #266392 - Flags: review?(roc)
This makes these properties apply to SVG elements. Do you know what they're going to return for SVG elements? Does it make sense?
Comment on attachment 266392 [details] [diff] [review]
support offset/client/scroll properties in xul

clearing request until question is addressed
Attachment #266392 - Flags: superreview?(roc)
Attachment #266392 - Flags: review?(roc)
Roc, there's a spec for this at http://dev.w3.org/csswg/cssom-view/ which indicates that these properties are html only using an HTMLElementView interface. Maybe I should put these properties in a new interface and just have XUL elements implement this interface as well.
I don't think we really want offset* for anything but HTML. That stuff is evil and must be stamped out.

Seems to me client* and scroll* are reasonably well defined and should apply to all elements. I'll mention that to Anne and see if we can get the spec updated.
Well, I mainly want to be able to remove the need for box objects. getBoundingClientRect has the problem that it returns floats which means you have to round them in many cases, and doesn't provide a means of retrieving the width/height.

So replacing elem.boxObject.width with much longer code doesn't seem logical.

Note that elem.clientWidth returns the padding width of a element (inside the scroll bars) making it not a suitable replacement.

> which means you have to round them in many cases

In what cases?

> So replacing elem.boxObject.width with much longer code doesn't seem logical.

Then we should add height and width properties to ClientRect.
Depends on: 430445
Is there any reason the second patch hasn't had review requested on it yet?
Component: XP Toolkit/Widgets: XUL → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
Comment on attachment 319807 [details] [diff] [review]
updated patch and includes tests for clientXXX, offsetXXX and scrollXXX properties

Yes, this can be reviewed.
Attachment #319807 - Flags: superreview?(roc)
Attachment #319807 - Flags: review?(roc)
The patch also adds the width and height properties to ClientRect which are currently in the spec at http://dev.w3.org/csswg/cssom-view/
The spec isn't clear bout what scroll* and client* should return for SVG elements, but I think they don't "have an associated CSS layout box" so they should all return zero. I think currently we return nonsense. Otherwise the patch is great, so just fix that and we're done.
Attachment #319807 - Attachment is obsolete: true
Attachment #332540 - Flags: superreview?(roc)
Attachment #332540 - Flags: review?(roc)
Attachment #319807 - Flags: superreview?(roc)
Attachment #319807 - Flags: review?(roc)
Attachment #332540 - Flags: superreview?(roc)
Attachment #332540 - Flags: superreview+
Attachment #332540 - Flags: review?(roc)
Attachment #332540 - Flags: review+
Attached patch changes (obsolete) — Splinter Review
OK, I needed to make some changes because it didn't seem to work on Windows anymore, but did a while ago.
Attachment #332540 - Attachment is obsolete: true
Attachment #332950 - Flags: superreview?(roc)
Attachment #332950 - Flags: review?(roc)
+static nsIFrame*
+GetStyledFrameFor(nsGenericElement* aElement)
+{
+  nsIFrame *frame = aElement->GetPrimaryFrame(Flush_Layout);
+
+  return (frame && frame->GetType() == nsGkAtoms::tableOuterFrame) ?
+    frame->GetFirstChild(nsnull) : frame;
+}

Can you share this with the equivalent code in nsComputedDOMStyle::GetPropertyCSSValue and possibly elsewhere? At least the part other than the Flush should be easy to share via an nsLayoutUtils method. You can share with nsGenericHTMLElement by making it a method on nsGenericElement.

+  aRect.x = aRect.y = 0;
+  aRect.Empty();

I'd just write aRect = nsRect();
(In reply to comment #22)
> +static nsIFrame*
> +GetStyledFrameFor(nsGenericElement* aElement)
> +{
> +  nsIFrame *frame = aElement->GetPrimaryFrame(Flush_Layout);
> +
> +  return (frame && frame->GetType() == nsGkAtoms::tableOuterFrame) ?
> +    frame->GetFirstChild(nsnull) : frame;
> +}
> 
> Can you share this with the equivalent code in
> nsComputedDOMStyle::GetPropertyCSSValue and possibly elsewhere?

I'm not clear on what should be shared here. GetStyledFrameFor? Just the last line? I haven't changed the code, just moved it.

I will however share GetStyledFrameFor between nsGenericElement and nsGenericHTMLElement
the return expression could be shared more widely, but it's OK to just share between nsGenericElement and nsGenericHTMLElement so we're not adding more duplication.
Attached patch fix issuesSplinter Review
Attachment #332950 - Attachment is obsolete: true
Attachment #333445 - Flags: superreview?(roc)
Attachment #333445 - Flags: review?(roc)
Attachment #332950 - Flags: superreview?(roc)
Attachment #332950 - Flags: review?(roc)
Attachment #333445 - Flags: superreview?(roc)
Attachment #333445 - Flags: superreview+
Attachment #333445 - Flags: review?(roc)
Attachment #333445 - Flags: review+
The microformat code is trying to do:

var box = microformatNodes[i].getBoundingClientRect();
box.width = box.right - box.left;
box.height = box.bottom - box.top;

which fails now because width and height are already defined.

Is some extra magic needed to support this, or should Microformat.js just be changed?
We can make this change anyway to fix the test.
Attachment #333977 - Flags: review?(sayrer)
Attachment #333977 - Flags: review?(sayrer) → review?(mozilla)
Comment on attachment 333977 [details] [diff] [review]
This would be the microformat fix

changing the test is fine. I only did that as a convenience so I would have width/height for both the getBoundingClientRect case and the getBoxObjectFor case.
Attachment #333977 - Flags: review?(mozilla) → review+
Should we make width and height replaceable properties?
Checked this in
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
Target Milestone: mozilla1.1alpha → mozilla1.9.1a2
Flags: in-testsuite+
(client|scroll)(Height|Width) should probably also show up here: http://developer.mozilla.org/En/XUL/Property
You need to log in before you can comment on or make changes to this bug.