Bug 14380 - Long DOM ancestry breadcrumb lists get cut off
Summary: Long DOM ancestry breadcrumb lists get cut off
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 523.x (Safari 3)
Hardware: All All
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-06-24 23:08 PDT by Adam Roben (:aroben)
Modified: 2007-11-14 11:21 PST (History)
0 users

See Also:


Attachments
Patch (17.49 KB, patch)
2007-11-14 09:41 PST, Timothy Hatcher
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 2007-06-24 23:08:58 PDT
Long DOM ancestry breadcrumb lists get cut off by the right sidebar in the Inspector's DOM view. We could fix this by collapsing the left end of the breadcrumb list when it gets too long, then expanding it when you mouse over the collapsed section.
Comment 1 Timothy Hatcher 2007-11-14 09:41:14 PST
Created attachment 17271 [details]
Patch
Comment 2 Adam Roben (:aroben) 2007-11-14 10:12:59 PST
Comment on attachment 17271 [details]
Patch

+            if (event.detail >= 2 || event.currentTarget.hasStyleClass("dimmed"))

I think the event.detail part of this line could use a comment.

+        // The order of the crumbs in the document is oposite of the visual order.

Typo: oposite -> opposite

+            for (var significantIndex = (crumbs.childNodes.length - 1); significantIndex >= 0; --significantIndex)

The parentheses around the subtraction aren't needed.

+            // Walk in towards the significant crumb by calculating the side that has the most crumbs
+            // and shrinking that first. Once both sides are equal, favor the shrinking the left side,
+            // then alternate sides.

This comment makes it sound like you are going to do 3 loops, but you only do one (which is a good thing). I think we can simplify the description a bit to something like "Shrink crumbs one at a time by applying the shrinkingFunction until the crumbs fit in the crumbsContainer or we run out of crumbs to shrink. Crumbs are shrunk in order of descending distance from the signifcant crumb, with a tie going to crumbs on the left."

+            var startInset = 0;
+            var endInset = 0;
+            var lastIndex = crumbs.childNodes.length - 1;
+            var startOffset = significantIndex;
+            var endOffset = (lastIndex - significantIndex);
+
+            while (startOffset > 0 || endOffset > 0) {
+                if (startOffset > endOffset) {
+                    // Shrink from the right side.
+                    var shrinkCrumb = crumbs.childNodes[startInset];
+                    ++startInset;
+                    startOffset = (significantIndex - startInset);
+                } else {
+                    // Shrink from the left side.
+                    var shrinkCrumb = crumbs.childNodes[(lastIndex - endInset)];
+                    ++endInset;
+                    endOffset = (lastIndex - significantIndex - endInset);
+                }

I think this could all be made a little simpler, like so:

var startIndex = 0;
var endIndex = crumbs.childNodes.length - 1;
while (startIndex != significantIndex || endIndex != significantIndex) {
    var startDistance = significantIndex - startIndex;
    var endDistance = endIndex - significantIndex;
    if (startDistance > endDistance) {
        var shrinkCrumb = crumbs.childNodes[startIndex];
        ++startIndex;
    } else {
        var shrinkCrumb = crumbs.childNodes[endIndex];
        --endIndex;
    }

+            if ((crumbs.offsetWidth + rightPadding) < crumbsContainer.offsetWidth)
+                return; // No need to compact the crumbs more.

Maybe you can put this check in a helper function, since you have to do it in so many places.

r=me
Comment 3 Timothy Hatcher 2007-11-14 11:21:49 PST
Landed in r27789. http://trac.webkit.org/projects/webkit/changeset/27789