You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Stephen Voorhees <st...@autodesk.com> on 2009/07/17 22:35:00 UTC
Bug in dynamic-height
Hi All,
I've been debugging an issue with dynamic-height on Safari & Chrome. On both of these browsers, we are seeing the height set to 0px rather than the gadget height on our container implementation. In debugging this I notice that the following method always returns 0:
function getHeightForWebkit() {
var result = 0;
var children = document.body.childNodes;
for (var i = 0; i < children.length; i++) {
if (children[i].offsetTop && children[i].offsetHeight) {
var bottom = children[i].offsetTop + children[i].scrollHeight
+ parseIntFromElemPxAttribute(children[i], "margin-bottom")
+ parseIntFromElemPxAttribute(children[i], "padding-bottom");
result = Math.max(result, bottom);
}
}
// Add margin and padding height specificed in the body (if any).
return result
}
For our test gadget, children[i].offsetTop = 0 (offsetHeight & scrollHeight = 1000), and therefore regardless of the value of offsetHeight, the following always resolves to false:
if (children[i].offsetTop && children[i].offsetHeight) {
Therefore, bottom is never defined and the method always returns 0, setting our gadget height to 0.
I'm not 100% sure how to fix it because I do not know the intent of the code. Should this check even be there? Should it be checking for 'undefined' instead? Also, why is it checking for children[i].offsetHeight yet then never using it?
Any ideas on how to fix this would be appreciated.
--Steve
Re: Bug in dynamic-height
Posted by Stephen Voorhees <st...@autodesk.com>.
JIRA issue created, patch attached to issue.
https://issues.apache.org/jira/browse/SHINDIG-1118
I tested this fix locally and it works well for Safari and Chrome.
On 7/17/09 2:10 PM, "John Hjelmstad" <fa...@google.com> wrote:
Yes and yes.
On Fri, Jul 17, 2009 at 2:08 PM, Stephen Voorhees <
stephen.voorhees@autodesk.com> wrote:
> Should it be:
>
> if (typeof children[i].offsetTop !== 'undefined' &&
> typeof children[i].scrollHeight !== 'undefined') {
>
> ?
>
> Do you need me to submit a patch?
>
> On 7/17/09 2:02 PM, "John Hjelmstad" <fa...@google.com> wrote:
>
> +1. I'd be happy to submit a fix.
>
> On Fri, Jul 17, 2009 at 1:54 PM, Michael Hermanto <mhermanto@google.com
> >wrote:
>
> > This code to traverse all the gadget children elements is needed because
> > the
> > old implementation will not allow a gadget's size to be reduced in
> Webkit.
> > I
> > believe either scrollHeight or offsetHeight (in the old code path) can
> > always be a larger number. I think the issue here is just with the
> > if-condition. It should have been --
> > if (children[i].offsetTop != undefined && children[i].scrollHeight !=
> > undefined) {
> >
> >
> > On Fri, Jul 17, 2009 at 1:35 PM, Stephen Voorhees <
> > stephen.voorhees@autodesk.com> wrote:
> >
> > > Hi All,
> > >
> > > I've been debugging an issue with dynamic-height on Safari & Chrome.
> On
> > > both of these browsers, we are seeing the height set to 0px rather than
> > the
> > > gadget height on our container implementation. In debugging this I
> > notice
> > > that the following method always returns 0:
> > >
> > > function getHeightForWebkit() {
> > > var result = 0;
> > > var children = document.body.childNodes;
> > > for (var i = 0; i < children.length; i++) {
> > > if (children[i].offsetTop && children[i].offsetHeight) {
> > > var bottom = children[i].offsetTop + children[i].scrollHeight
> > > + parseIntFromElemPxAttribute(children[i], "margin-bottom")
> > > + parseIntFromElemPxAttribute(children[i], "padding-bottom");
> > > result = Math.max(result, bottom);
> > > }
> > > }
> > > // Add margin and padding height specificed in the body (if any).
> > > return result
> > > }
> > >
> > > For our test gadget, children[i].offsetTop = 0 (offsetHeight &
> > scrollHeight
> > > = 1000), and therefore regardless of the value of offsetHeight, the
> > > following always resolves to false:
> > >
> > > if (children[i].offsetTop && children[i].offsetHeight) {
> > >
> > > Therefore, bottom is never defined and the method always returns 0,
> > setting
> > > our gadget height to 0.
> > >
> > > I'm not 100% sure how to fix it because I do not know the intent of the
> > > code. Should this check even be there? Should it be checking for
> > > 'undefined' instead? Also, why is it checking for
> > children[i].offsetHeight
> > > yet then never using it?
> > >
> > > Any ideas on how to fix this would be appreciated.
> > >
> > > --Steve
> > >
> >
> >
> >
> > --
> > mh x31813
> >
>
>
Re: Bug in dynamic-height
Posted by John Hjelmstad <fa...@google.com>.
Yes and yes.
On Fri, Jul 17, 2009 at 2:08 PM, Stephen Voorhees <
stephen.voorhees@autodesk.com> wrote:
> Should it be:
>
> if (typeof children[i].offsetTop !== 'undefined' &&
> typeof children[i].scrollHeight !== 'undefined') {
>
> ?
>
> Do you need me to submit a patch?
>
> On 7/17/09 2:02 PM, "John Hjelmstad" <fa...@google.com> wrote:
>
> +1. I'd be happy to submit a fix.
>
> On Fri, Jul 17, 2009 at 1:54 PM, Michael Hermanto <mhermanto@google.com
> >wrote:
>
> > This code to traverse all the gadget children elements is needed because
> > the
> > old implementation will not allow a gadget's size to be reduced in
> Webkit.
> > I
> > believe either scrollHeight or offsetHeight (in the old code path) can
> > always be a larger number. I think the issue here is just with the
> > if-condition. It should have been --
> > if (children[i].offsetTop != undefined && children[i].scrollHeight !=
> > undefined) {
> >
> >
> > On Fri, Jul 17, 2009 at 1:35 PM, Stephen Voorhees <
> > stephen.voorhees@autodesk.com> wrote:
> >
> > > Hi All,
> > >
> > > I've been debugging an issue with dynamic-height on Safari & Chrome.
> On
> > > both of these browsers, we are seeing the height set to 0px rather than
> > the
> > > gadget height on our container implementation. In debugging this I
> > notice
> > > that the following method always returns 0:
> > >
> > > function getHeightForWebkit() {
> > > var result = 0;
> > > var children = document.body.childNodes;
> > > for (var i = 0; i < children.length; i++) {
> > > if (children[i].offsetTop && children[i].offsetHeight) {
> > > var bottom = children[i].offsetTop + children[i].scrollHeight
> > > + parseIntFromElemPxAttribute(children[i], "margin-bottom")
> > > + parseIntFromElemPxAttribute(children[i], "padding-bottom");
> > > result = Math.max(result, bottom);
> > > }
> > > }
> > > // Add margin and padding height specificed in the body (if any).
> > > return result
> > > }
> > >
> > > For our test gadget, children[i].offsetTop = 0 (offsetHeight &
> > scrollHeight
> > > = 1000), and therefore regardless of the value of offsetHeight, the
> > > following always resolves to false:
> > >
> > > if (children[i].offsetTop && children[i].offsetHeight) {
> > >
> > > Therefore, bottom is never defined and the method always returns 0,
> > setting
> > > our gadget height to 0.
> > >
> > > I'm not 100% sure how to fix it because I do not know the intent of the
> > > code. Should this check even be there? Should it be checking for
> > > 'undefined' instead? Also, why is it checking for
> > children[i].offsetHeight
> > > yet then never using it?
> > >
> > > Any ideas on how to fix this would be appreciated.
> > >
> > > --Steve
> > >
> >
> >
> >
> > --
> > mh x31813
> >
>
>
Re: Bug in dynamic-height
Posted by Stephen Voorhees <st...@autodesk.com>.
Should it be:
if (typeof children[i].offsetTop !== 'undefined' &&
typeof children[i].scrollHeight !== 'undefined') {
?
Do you need me to submit a patch?
On 7/17/09 2:02 PM, "John Hjelmstad" <fa...@google.com> wrote:
+1. I'd be happy to submit a fix.
On Fri, Jul 17, 2009 at 1:54 PM, Michael Hermanto <mh...@google.com>wrote:
> This code to traverse all the gadget children elements is needed because
> the
> old implementation will not allow a gadget's size to be reduced in Webkit.
> I
> believe either scrollHeight or offsetHeight (in the old code path) can
> always be a larger number. I think the issue here is just with the
> if-condition. It should have been --
> if (children[i].offsetTop != undefined && children[i].scrollHeight !=
> undefined) {
>
>
> On Fri, Jul 17, 2009 at 1:35 PM, Stephen Voorhees <
> stephen.voorhees@autodesk.com> wrote:
>
> > Hi All,
> >
> > I've been debugging an issue with dynamic-height on Safari & Chrome. On
> > both of these browsers, we are seeing the height set to 0px rather than
> the
> > gadget height on our container implementation. In debugging this I
> notice
> > that the following method always returns 0:
> >
> > function getHeightForWebkit() {
> > var result = 0;
> > var children = document.body.childNodes;
> > for (var i = 0; i < children.length; i++) {
> > if (children[i].offsetTop && children[i].offsetHeight) {
> > var bottom = children[i].offsetTop + children[i].scrollHeight
> > + parseIntFromElemPxAttribute(children[i], "margin-bottom")
> > + parseIntFromElemPxAttribute(children[i], "padding-bottom");
> > result = Math.max(result, bottom);
> > }
> > }
> > // Add margin and padding height specificed in the body (if any).
> > return result
> > }
> >
> > For our test gadget, children[i].offsetTop = 0 (offsetHeight &
> scrollHeight
> > = 1000), and therefore regardless of the value of offsetHeight, the
> > following always resolves to false:
> >
> > if (children[i].offsetTop && children[i].offsetHeight) {
> >
> > Therefore, bottom is never defined and the method always returns 0,
> setting
> > our gadget height to 0.
> >
> > I'm not 100% sure how to fix it because I do not know the intent of the
> > code. Should this check even be there? Should it be checking for
> > 'undefined' instead? Also, why is it checking for
> children[i].offsetHeight
> > yet then never using it?
> >
> > Any ideas on how to fix this would be appreciated.
> >
> > --Steve
> >
>
>
>
> --
> mh x31813
>
Re: Bug in dynamic-height
Posted by John Hjelmstad <fa...@google.com>.
+1. I'd be happy to submit a fix.
On Fri, Jul 17, 2009 at 1:54 PM, Michael Hermanto <mh...@google.com>wrote:
> This code to traverse all the gadget children elements is needed because
> the
> old implementation will not allow a gadget's size to be reduced in Webkit.
> I
> believe either scrollHeight or offsetHeight (in the old code path) can
> always be a larger number. I think the issue here is just with the
> if-condition. It should have been --
> if (children[i].offsetTop != undefined && children[i].scrollHeight !=
> undefined) {
>
>
> On Fri, Jul 17, 2009 at 1:35 PM, Stephen Voorhees <
> stephen.voorhees@autodesk.com> wrote:
>
> > Hi All,
> >
> > I've been debugging an issue with dynamic-height on Safari & Chrome. On
> > both of these browsers, we are seeing the height set to 0px rather than
> the
> > gadget height on our container implementation. In debugging this I
> notice
> > that the following method always returns 0:
> >
> > function getHeightForWebkit() {
> > var result = 0;
> > var children = document.body.childNodes;
> > for (var i = 0; i < children.length; i++) {
> > if (children[i].offsetTop && children[i].offsetHeight) {
> > var bottom = children[i].offsetTop + children[i].scrollHeight
> > + parseIntFromElemPxAttribute(children[i], "margin-bottom")
> > + parseIntFromElemPxAttribute(children[i], "padding-bottom");
> > result = Math.max(result, bottom);
> > }
> > }
> > // Add margin and padding height specificed in the body (if any).
> > return result
> > }
> >
> > For our test gadget, children[i].offsetTop = 0 (offsetHeight &
> scrollHeight
> > = 1000), and therefore regardless of the value of offsetHeight, the
> > following always resolves to false:
> >
> > if (children[i].offsetTop && children[i].offsetHeight) {
> >
> > Therefore, bottom is never defined and the method always returns 0,
> setting
> > our gadget height to 0.
> >
> > I'm not 100% sure how to fix it because I do not know the intent of the
> > code. Should this check even be there? Should it be checking for
> > 'undefined' instead? Also, why is it checking for
> children[i].offsetHeight
> > yet then never using it?
> >
> > Any ideas on how to fix this would be appreciated.
> >
> > --Steve
> >
>
>
>
> --
> mh x31813
>
Re: Bug in dynamic-height
Posted by Michael Hermanto <mh...@google.com>.
This code to traverse all the gadget children elements is needed because the
old implementation will not allow a gadget's size to be reduced in Webkit. I
believe either scrollHeight or offsetHeight (in the old code path) can
always be a larger number. I think the issue here is just with the
if-condition. It should have been --
if (children[i].offsetTop != undefined && children[i].scrollHeight !=
undefined) {
On Fri, Jul 17, 2009 at 1:35 PM, Stephen Voorhees <
stephen.voorhees@autodesk.com> wrote:
> Hi All,
>
> I've been debugging an issue with dynamic-height on Safari & Chrome. On
> both of these browsers, we are seeing the height set to 0px rather than the
> gadget height on our container implementation. In debugging this I notice
> that the following method always returns 0:
>
> function getHeightForWebkit() {
> var result = 0;
> var children = document.body.childNodes;
> for (var i = 0; i < children.length; i++) {
> if (children[i].offsetTop && children[i].offsetHeight) {
> var bottom = children[i].offsetTop + children[i].scrollHeight
> + parseIntFromElemPxAttribute(children[i], "margin-bottom")
> + parseIntFromElemPxAttribute(children[i], "padding-bottom");
> result = Math.max(result, bottom);
> }
> }
> // Add margin and padding height specificed in the body (if any).
> return result
> }
>
> For our test gadget, children[i].offsetTop = 0 (offsetHeight & scrollHeight
> = 1000), and therefore regardless of the value of offsetHeight, the
> following always resolves to false:
>
> if (children[i].offsetTop && children[i].offsetHeight) {
>
> Therefore, bottom is never defined and the method always returns 0, setting
> our gadget height to 0.
>
> I'm not 100% sure how to fix it because I do not know the intent of the
> code. Should this check even be there? Should it be checking for
> 'undefined' instead? Also, why is it checking for children[i].offsetHeight
> yet then never using it?
>
> Any ideas on how to fix this would be appreciated.
>
> --Steve
>
--
mh x31813