The default bug view has changed. See this FAQ.

"No scripts." should be displayed in the dropdown when there are no scripts matching the search string

RESOLVED FIXED in Firefox 17

Status

()

Firefox
Developer Tools: Debugger
P3
normal
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ioana (away), Assigned: vporof)

Tracking

15 Branch
Firefox 17
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [testday-20120706])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Mozilla/5.0 (Windows NT 6.1; rv:15.0) Gecko/20120705 Firefox/15.0a2 (20120705042010)

STR:
1. Load a web page with multiple scripts in the browser.
2. Open the Debugger.
3. Enter a string that doesn't match any of the scripts' names in the "Filter scripts" textbox.

"No scripts." should be displayed in the dropdown since there are no scripts matching the search string. Instead of this, the last selected script is displayed.
(Reporter)

Updated

5 years ago
Whiteboard: [testday-20120706]
Victor, wanna take this? Should probably read "No matching scripts" or something.
Priority: -- → P3
(Assignee)

Comment 2

5 years ago
Okay.
Assignee: nobody → vporof
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Created attachment 643261 [details] [diff] [review]
v1
Attachment #643261 - Flags: review?(past)
(Assignee)

Updated

5 years ago
Blocks: 774220
(Assignee)

Comment 4

5 years ago
Created attachment 643269 [details] [diff] [review]
v2

On this note, we should also display "No scripts" after navigating to a page with no scripts, instead of leaving the menulist completely naked like we do now.
Added that + more assertions in tests.
Attachment #643261 - Attachment is obsolete: true
Attachment #643261 - Flags: review?(past)
Attachment #643269 - Flags: review?(past)
Comment on attachment 643269 [details] [diff] [review]
v2

Review of attachment 643269 [details] [diff] [review]:
-----------------------------------------------------------------

A few minor issues. r=me with those.
I'd have set the flag to r+ if bugzilla would let me.

::: browser/devtools/debugger/debugger-view.js
@@ +483,5 @@
>            item.hidden = true;
>          }
>        }
> +      if (!found) {
> +        scripts.setAttribute("label", L10N.getStr("noMatchingScriptsText"));

You need to set the tooltip to the same string as well, because otherwise it retains any previously held value. If the string was displayed in full of course, clearing the tooltip would suffice.

::: browser/locales/en-US/chrome/browser/devtools/debugger.properties
@@ +60,5 @@
>  emptyBreakpointsText=No breakpoints to display.
>  
> +# LOCALIZATION NOTE (noScriptsText): The text to display in the menulist when
> +# there are no scripts.
> +noScriptsText=No scripts.

The full stop doesn't look good in a menu list I think.

@@ +64,5 @@
> +noScriptsText=No scripts.
> +
> +# LOCALIZATION NOTE (noMatchingScriptsText): The text to display in the
> +# menulist when there are no matching scripts after filtering.
> +noMatchingScriptsText=No matching scripts.

Ditto.

::: browser/themes/gnomestripe/devtools/debugger.css
@@ +18,4 @@
>  
>  #scripts {
>    max-width: 350px;
> +  min-width: 150px;

Can we get this big enough to hold the 'no match' string without an ellipsis?
Comment on attachment 643269 [details] [diff] [review]
v2

past - bugzilla: 1 - 1
Attachment #643269 - Flags: review?(past) → review+
(Assignee)

Comment 7

5 years ago
(In reply to Panos Astithas [:past] from comment #5)
> Comment on attachment 643269 [details] [diff] [review]
> v2
> 
> Review of attachment 643269 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> A few minor issues. r=me with those.
> I'd have set the flag to r+ if bugzilla would let me.
> 
> ::: browser/devtools/debugger/debugger-view.js
> @@ +483,5 @@
> >            item.hidden = true;
> >          }
> >        }
> > +      if (!found) {
> > +        scripts.setAttribute("label", L10N.getStr("noMatchingScriptsText"));
> 
> You need to set the tooltip to the same string as well, because otherwise it
> retains any previously held value. If the string was displayed in full of
> course, clearing the tooltip would suffice.

You're right.
How about just clearing the tooltiptext though, since having the same tooltip as the contents is not really necessary?
(In reply to Victor Porof from comment #7)
> (In reply to Panos Astithas [:past] from comment #5)
> > Comment on attachment 643269 [details] [diff] [review]
> > v2
> > 
> > Review of attachment 643269 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > A few minor issues. r=me with those.
> > I'd have set the flag to r+ if bugzilla would let me.
> > 
> > ::: browser/devtools/debugger/debugger-view.js
> > @@ +483,5 @@
> > >            item.hidden = true;
> > >          }
> > >        }
> > > +      if (!found) {
> > > +        scripts.setAttribute("label", L10N.getStr("noMatchingScriptsText"));
> > 
> > You need to set the tooltip to the same string as well, because otherwise it
> > retains any previously held value. If the string was displayed in full of
> > course, clearing the tooltip would suffice.
> 
> You're right.
> How about just clearing the tooltiptext though, since having the same
> tooltip as the contents is not really necessary?

Sure. Like I said, if the string is legible, clearing the tooltip is sufficient. Right now all I see is "No ma...".
(Assignee)

Comment 9

5 years ago
Created attachment 643345 [details] [diff] [review]
v2.1

Addressed comments.
Fixing the menupopup sizing issue by adding a "sizetopopup" = always attribute.
(Assignee)

Updated

5 years ago
Attachment #643345 - Attachment is patch: true
(Assignee)

Updated

5 years ago
Whiteboard: [testday-20120706] → [testday-20120706][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/c55c28ca8d04
Whiteboard: [testday-20120706][land-in-fx-team] → [testday-20120706][fixed-in-fx-team]
Attachment #643269 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/c55c28ca8d04
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [testday-20120706][fixed-in-fx-team] → [testday-20120706]
Target Milestone: --- → Firefox 17
You need to log in before you can comment on or make changes to this bug.