2011-02-08 Ojan Vafai <ojan@chromium.org>
authorojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Feb 2011 06:42:36 +0000 (06:42 +0000)
committerojan@chromium.org <ojan@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Feb 2011 06:42:36 +0000 (06:42 +0000)
        Reviewed by Adam Barth.

        fix toolbar anchoring in the code review tool
        https://bugs.webkit.org/show_bug.cgi?id=54058

        Avoid the anchoring cycle of doom when on the cusp
        of whether the toolbar needs to be anchored and
        speculatively avoid the Firefox crash when resizing.

        * PrettyPatch/PrettyPatch.rb:
        * code-review.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@78019 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Websites/bugs.webkit.org/ChangeLog
Websites/bugs.webkit.org/PrettyPatch/PrettyPatch.rb
Websites/bugs.webkit.org/code-review.js

index 0da5c66..fced929 100644 (file)
@@ -1,3 +1,17 @@
+2011-02-08  Ojan Vafai  <ojan@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        fix toolbar anchoring in the code review tool
+        https://bugs.webkit.org/show_bug.cgi?id=54058
+
+        Avoid the anchoring cycle of doom when on the cusp
+        of whether the toolbar needs to be anchored and
+        speculatively avoid the Firefox crash when resizing.
+
+        * PrettyPatch/PrettyPatch.rb:
+        * code-review.js:
+
 2011-02-07  Ojan Vafai  <ojan@chromium.org>
 
         Reviewed by Adam Barth.
index 3ca5db3..ad22bf5 100644 (file)
@@ -275,16 +275,16 @@ body {
   display: -webkit-box;
   display: -moz-box;
   padding: 3px;
-  bottom: 0;
   left: 0;
   right: 0;
   border: 1px solid #ddd;
   background-color: #eee;
   font-family: sans-serif;
+  position: fixed;
 }
 
 #toolbar.anchored {
-  position: fixed;
+  bottom: 0;
 }
 
 #toolbar .actions {
index ed6cb09..dc68ad8 100644 (file)
@@ -981,8 +981,12 @@ var CODE_REVIEW_UNITTEST;
   }
 
   function updateToolbarAnchorState() {
-    var has_scrollbar = window.innerWidth > document.documentElement.offsetWidth;
-    $('#toolbar').toggleClass('anchored', has_scrollbar);
+    var toolbar = $('#toolbar');
+    // Unanchor the toolbar and then see if it's bottom is below the body's bottom.
+    toolbar.toggleClass('anchored', false);
+    var toolbar_bottom = toolbar.offset().top + toolbar.outerHeight();
+    var should_anchor = toolbar_bottom >= document.body.clientHeight;
+    toolbar.toggleClass('anchored', should_anchor);
   }
 
   function diffLinksHtml() {
@@ -1010,7 +1014,7 @@ var CODE_REVIEW_UNITTEST;
               '<span id="flagContainer"></span>' +
               '<button id="preview_comments">Preview</button>' +
               '<button id="post_comments">Publish</button> ' +
-          '</span></div>' +
+          '</span>' +
         '</div>' +
         '</div>');
 
@@ -1023,7 +1027,8 @@ var CODE_REVIEW_UNITTEST;
     // FIXME: Should we setTimeout throttle these?
     var resize_iframe = $('<iframe class="pseudo_resize_event_iframe"></iframe>');
     $(document.body).append(resize_iframe);
-    $(resize_iframe[0].contentWindow).bind('resize', onBodyResize);
+    // Handle the event on a timeout to avoid crashing Firefox.
+    $(resize_iframe[0].contentWindow).bind('resize', function() { setTimeout(onBodyResize, 0)});
 
     updateToolbarAnchorState();
     loadDiffState();