Prototype

Comments and changes to this ticket

• 

  Tobie Langel July 1st, 2008 @ 10:17 AM

  I'd suggest either a proper sniff:

        var isBuggy = Prototype.Browser.IE && element.innerHTML.blank();
        if (isBuggy) element.innerHTML = '\x00';
  

  or, if it isn't a perf bottleneck, why not do it for empty elements in every browser and thus avoid the sniffing altogether ?

  Also, is the issue with empty elements, or blank elements?

• 

  John-David Dalton July 1st, 2008 @ 02:53 PM

  Ah good point, what about image elements and other self closing elements.

  I saw the issue when working with divs so ya that check should be changed.

• 

  John-David Dalton July 1st, 2008 @ 03:41 PM

  Also using get Element.getStyle(element, 'width') is problematic on elements that have a value of "auto" because prototype uses offsetWidth or offsetHeight which includes padding and border width :(

• 

  John-David Dalton July 1st, 2008 @ 06:54 PM

  • → Title changed from “Code review of Element#relativize() (git core)” to “Code review of Element#relativize() (git core found bugs)”
  • → State changed from “new” to “bug”
  • → Tag changed from “” to “patched”

  On further reivew of Element#relativize and Element#absolutize I found some pretty big issues.

  
   Element#absolutize:
    1) calculated dimensions of the element (which include padding and border width),
       and then assigned that as the element.style.width/height (making it bigger than    
       needed)
   
   Element#relativize:
    1) there is no real accurate way to "automagically" guess the _original values from    
       elements that havent been called by Element#absolutize first.
       I took some code from Andrews dimensions.js and ironed out the other issues.
  
  
   Ideally the "relativize" and "absolutize" would be moved a little further
   down in the code so they could use the "getNumericStyle" private helper method.
  
   Also some unit tests for Element#absolutize with elements with borders and padding
   would be a big plus.
  

• 

  Nick Stakenburg July 6th, 2008 @ 02:26 PM

  • → Tag changed from “patched” to “needs_tests patched”

  Added unit tests to make sure dimensions stay untouched after using Element#absolutize|relativize.

  • 0001-Unit-tests-for-Element-absolutize-relativize-making.patch 4.4 KB
• 

  John-David Dalton July 18th, 2008 @ 04:58 PM

  uploaded a revised patch and unit tests

• 

  Nick Stakenburg July 18th, 2008 @ 11:54 PM

  • → Tag changed from “needs_tests patched” to “patched”

  Why throw that error in Element#relativize? Seems to me like an element with position:absolute can never be relativized using that patch. Calling Element#absolutize on a position:absolute element won't set _original values.

• 

  John-David Dalton July 19th, 2008 @ 05:10 AM

  I am following the code from Andrew’s fork because there seems to be no accurate way to address
  the issue of going from an absolute positioned element to a relatively positioned element.
  Issue arise when you try to position the element around the possible other child elements and 
  dealing with the sizing limitations of a parent element. 
  
  I also had to modify your unit tests a bit because they were causing false failures.
  
  If you can tweak the patch to work for both absolute and relative please do.
  I did this after running into a brick wall on the previous approach.
  
  1.6.0.2 causes an operational error in IE and possible others when you try to run 
  Element#relativize without running Element#absolutize first.
  At least patch informs the user of the issue.
  

• 

  Nick Stakenburg July 19th, 2008 @ 09:01 AM

  At least patch informs the user of the issue.

  It would if in the case of an absolute element the error was:

  'Elements with position absolute cannot be relativized'.

  Having Element#relativize that doesn't work on absolute elements seems pointless though.

• 

  John-David Dalton July 19th, 2008 @ 04:50 PM

  Element#relativize is just a mechanism to undo the absolutize of an element. Its not made to be called first.

  Like I said before though, you are totally more than welcome to fix this if ya want.

• 

  Nick Stakenburg July 19th, 2008 @ 05:27 PM

  Ah, that explains. The API makes it look like relativize is more then just a restore mechanism for absolutize.

  http://prototypejs.org/api/eleme...

  Besides restoring absolutize, I can't think of any real usecase for relativize, even if it was fixed. I'd rather not fix something nobody will use.

  Some documentation on Element#relativize being a restore mechanism for absolutize might be good though.

• 

  John-David Dalton July 19th, 2008 @ 05:53 PM

  Ya I agree the docs need to clarify its usage.

• 

  Christophe Porteneuve July 27th, 2008 @ 07:25 PM

  Woah, you know, if relativize actually is a "undo" for @absolutize@, we should use consistent naming and say makeAbsolute and @undoAbsolute@, like we do for …Positioned and …Clipping.

  My revised FR book also states 1.6.0.3 will fix the padding/border issue, so let's make it happen! :-)

• 

  John-David Dalton July 28th, 2008 @ 05:25 AM

  Here is a simple test with padding margins and borders thrown in for fun :) (using the revised patch to the right)

  <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
          "http://www.w3.org/TR/xhtml1/DTD/...">
  <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
  <head>
  <title>Test</title>
  <meta http-equiv="Content-Type" content="text/html;charset=utf-8">
  
  <style>
  #container {
   border: 13px solid gray;
   padding: 13px;
   height:200px;
   width:500px;
   margin-left:5px;
   maring-top: -2px;
  }
  #foo {
    position: relative;
    border: 3px solid blue;
    padding: 2px;
    z-index: 1;
    width: 100%;
    height:50%;
    margin-left: -10px;
    margin-top: 5px;
  }
  </style>
  
  <script src="prototype.js" type="text/javascript"></script>
  <script type="text/javascript">
  
  document.observe('dom:loaded', function() {
    alert('before');
    $('foo').absolutize();
    alert('after');
    $('foo').relativize();
    alert('back');
  }); 
   
  </script>
  
  </head>
  <body>
  
  <div id="container">
    <div id="foo">Hello</div>
  </div>
  
  </body>
  </html>
  

• 

  John-David Dalton August 17th, 2008 @ 09:09 PM

  made the unit test not appear over the test results.

  • _revised-Unit-tests-for-Element-absolutive-and-Element-relati.patch 4.8 KB
  • _revised-Element-absolute-and-Element-relativize.patch 6.3 KB