Closed Bug 777093 Opened 9 years ago Closed 9 years ago

Long script urls still confuse the debugger menulist


(DevTools :: Debugger, defect, P2)

12 Branch


(Not tracked)

Firefox 17


(Reporter: vporof, Assigned: vporof)



(1 file, 1 obsolete file)

Yup, regression from patch in bug 771481 using sizetopopup="always" on the menulist.
Attached patch v1Splinter Review
Asking Rob in for review.
Attachment #645712 - Flags: review?(rcampbell)
Comment on attachment 645712 [details] [diff] [review]

+const SCRIPTS_URL_MAX_LENGTH = 64; // chars

magic numbers! How did you arrive at 64? It's a nice power of two, a square and a cube. Its digits add up to 10. It has many extremely nice properties. But as a length of characters in a menu list?

+      ok(gDebugger.DebuggerView.Scripts.containsLabel(nanana + "Batman!" + ellipsis),
+        "Script (15) label is incorrect.");

haha. "wat"
Attachment #645712 - Flags: review?(rcampbell) → review+
(In reply to Rob Campbell [:rc] (:robcee) from comment #3)
> Comment on attachment 645712 [details] [diff] [review]
> v1
> +const SCRIPTS_URL_MAX_LENGTH = 64; // chars

In addition to your valid arguments, 20 is dividable by 2 and 5, looks good and
(new Array(20).join(NaN) + "Batman!").length === 64.

In all seriousness, it feels like an acceptable number of chars to use and keep the menulist at an acceptable size. If you can think of other good algorithms to find a better number, please share.
nah, is cool.
Whiteboard: [land-in-fx-team]
Attached patch fail (obsolete) — Splinter Review
Comment on attachment 647971 [details] [diff] [review]

Wrong bug!
Attachment #647971 - Attachment description: [land-in-fx-team] → fail
Attachment #647971 - Attachment is obsolete: true
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 17

Regression :( Tp5 No Network Row Major MozAfterPaint increase 12.4% on Linux Fx-Team
    Previous: avg 243.519 stddev 16.676 of 30 runs up to revision fcb3934935e2
    New     : avg 273.667 stddev 2.315 of 5 runs since revision 834a0c1ef40b
    Change  : +30.148 (12.4% / z=1.808)
    Graph   :
I think from the graph it is evident that this regression is caused by the merge from m-c in changeset 8c63e260c4fa, which is conspicuously absent from the graph, but occurred right in the upward slope between revs f08a7ecc6d9f and 4f39921f782f.

This is way too low-level to be affected by debugger UI patches.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.