Maheshjayachandran / closure-library

Automatically exported from code.google.com/p/closure-library
0 stars 0 forks source link

goog.style.getBounds inconsistent for display:none element #415

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
goog.style.getBounds returns an inconsistent value when called on an element 
that is styled display:none element.

This is because it combines the results of goog.style.getPageOffset with 
goog.style.getSize.
*) goog.style.getPageOffset returns all zero for a display:none element
*) goog.style.getSize temporarily sets the element to display:inline, so it 
does not return all zero

Ideally goog.style.getPageOffset or goog.style.getBounds would also temporarily 
set the display style property.

Original issue reported on code.google.com by pdonelan@google.com on 1 Feb 2012 at 7:25

GoogleCodeExporter commented 9 years ago
We've recently been rejecting CLs that try to do this. There are a bunch of 
problems with the approach that getSize uses.
- It causes big performance regressions, because it forces the browser to 
re-layout the page extra times
- The spurious DOM modifications confuse things like goog.editor, which listen 
for dom modifications.
- Doing this to every function that reads the DOM would add a lot of code-side 
overhead.
- diplay: none is not overridable by descendents (i.e., if A is a parent of B 
and A has display: none, then there is nothing you can do to B to make it 
display. You have to climb the ancestor chain)

I think the better approach would be to have some sort of composable function 
like
var bounds = goog.style.withDisplayInline(el, function() {
  return goog.style.getBounds(el);
});

so that people would have to opt-in to this behavior.

Original comment by Nicholas.J.Santos on 2 Feb 2012 at 4:43

GoogleCodeExporter commented 9 years ago
In that case it feels like we should either:
a) document goog.style.getBounds to say that it returns invalid results for 
display:none elements, or
b) explicitly check for display:none and return all zeroes
Which would you prefer in a patch?

Original comment by pdonelan@google.com on 2 Feb 2012 at 3:43

GoogleCodeExporter commented 9 years ago
i don't have a strong opinion either way.

Original comment by Nicholas.J.Santos on 3 Feb 2012 at 12:27