777093 - Long script urls still confuse the debugger menulist

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, P2)

Description

[Victor Porof [:vporof][:vp]]

https://www.google.com/uds/api/gdata/1.10/208a907929e7936990d2bf48205aa8d8/core,mediarss,opensearch,geo,atom,threading,app,gdata,maps,gbase,finance,blogger,contacts,analytics,acl,calendar,default.I.js

is long.

Comment 1

[Victor Porof [:vporof][:vp]]

Yup, regression from patch in bug 771481 using sizetopopup="always" on the menulist.

Comment 2

[Victor Porof [:vporof][:vp]]

Attachment: v1

Asking Rob in for review.

Comment 3

[Rob Campbell [:rc] (:robcee)]

Comment on attachment 645712 [details] [diff] [review]
v1

+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"

Comment 4

[Victor Porof [:vporof][:vp]]

(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.
Sorry.

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.

Comment 5

[Rob Campbell [:rc] (:robcee)]

nah, is cool.

Comment 6

[Victor Porof [:vporof][:vp]]

Attachment: fail

Comment 7

[Victor Porof [:vporof][:vp]]

Comment on attachment 647971 [details] [diff] [review]
fail

Wrong bug!

Comment 8

[Victor Porof [:vporof][:vp]]

https://hg.mozilla.org/integration/fx-team/rev/3e77bd3b1596

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/3e77bd3b1596

Comment 10

[Ed Morley [:emorley]]

https://groups.google.com/d/topic/mozilla.dev.tree-management/2jB_s0SpJ80/discussion

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   : http://mzl.la/QmD6Pd

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

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.