[Webkit-unassigned] [Bug 45114] Web Inspector: show status message below call stack when debugger is paused

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 2 11:48:10 PDT 2010


https://bugs.webkit.org/show_bug.cgi?id=45114





--- Comment #3 from Pavel Podivilov <podivilov at chromium.org>  2010-09-02 11:48:09 PST ---
(In reply to comment #2)
> (From update of attachment 66383 [details])
> Great idea! Some quick observations on the patch.
> 
> > General Comments
> 
> Missing a ChangeLog!
> 
> 
> > diff --git WebCore/inspector/InspectorDebuggerAgent.cpp
> > -    details->setValue("reason", m_breakProgramReason);
> > +    details->setValue("status", m_breakProgramReason);
> 
> Did anything use "reason" before? I did a search and didn't see anything.
No, it actually was not used before. It was added at backend side, but will be used by fronted only after this change. BTW, this patch is based on another one, which is not landed yet (see https://bugs.webkit.org/show_bug.cgi?id=44679), sorry for confusion.

> 
> > diff WebCore/inspector/front-end/CallStackSidebarPane.js
> > +  format = WebInspector.UIString("Paused on \"%s\" breakpoint set on %s, because new child was added to it's descendant %s.");
> > +  format = WebInspector.UIString("Paused on \"%s\" breakpoint set on %s, because new child was added to that node.");
> > +  format = WebInspector.UIString("Paused on \"%s\" breakpoint set on %s, because it's descendant %s was removed.");
> > +  format = WebInspector.UIString("Paused on \"%s\" breakpoint set on %s.");
> 
> These look like new UIStrings. They should be added to WebCore/English.lproj/localizedStrings.js.
> My guess is that they were added, but your git diff didn't add the binary delta. When you
> create a diff to put up for review make sure you pass git diff the --binary switch to
> include binary file deltas.
Let me update localizedStrings.js when status messages are approved (corrections from native speakers are welcome).

> 
> 
> > +        var formatters = {
> > +            s: function(substitution)
> > +            {
> > +                return substitution;
> > +            }
> > +        };
> > +        function append(a, b)
> > +        {
> > +            if (typeof b === "string")
> > +                b = document.createTextNode(b);
> > +            statusElement.appendChild(b);
> > +        }
> > +        String.format(format, substitutions, formatters, "", append);
> 
> I'm sorry for not investigating this on my own. What is the reason
> for this using a custom formatter? Can you use the standard formatter
> found in utilities.js?
> 
>     String.standardFormatters
Standard "s" formatter checks substitution type, and if it's an object, returns substitution.description, which is inappropriate here.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list