Last Comment Bug 695279 - Make the script debugger UI l10n-friendly
: Make the script debugger UI l10n-friendly
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Panos Astithas [:past] (away until 7/21)
:
Mentors:
Depends on: 694538
Blocks: minotaur 692405 697762
  Show dependency treegraph
 
Reported: 2011-10-18 01:10 PDT by Panos Astithas [:past] (away until 7/21)
Modified: 2011-11-16 04:31 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (14.91 KB, patch)
2011-11-07 09:50 PST, Panos Astithas [:past] (away until 7/21)
no flags Details | Diff | Splinter Review
Working patch (21.40 KB, patch)
2011-11-08 02:45 PST, Panos Astithas [:past] (away until 7/21)
no flags Details | Diff | Splinter Review
Working patch v2 (21.40 KB, patch)
2011-11-08 04:02 PST, Panos Astithas [:past] (away until 7/21)
dcamp: review+
Details | Diff | Splinter Review

Description Panos Astithas [:past] (away until 7/21) 2011-10-18 01:10:03 PDT
The debugger UI must be properly internationalized like the rest of the developer tools. GCLI might be a good inspiration as it is also HTML-based. The proper location of the text files will be determined in accordance to bug 687851.
Comment 1 Panos Astithas [:past] (away until 7/21) 2011-11-07 05:33:04 PST
This bug required another fx-team merge, in order to get the new l10n bits:

https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/f8176b9f68db
Comment 2 Panos Astithas [:past] (away until 7/21) 2011-11-07 09:50:51 PST
Created attachment 572506 [details] [diff] [review]
WIP

Converted everything but the iframe to get strings from resource files. This will need to be converted to xhtml AFAICT and I'll get to it next.
Comment 3 Dave Camp (:dcamp) 2011-11-07 09:52:29 PST
In bug 700036 we started moving our xhtml/html documents to xul-documents-with-html-as-the-default-namespace.  This helps solve a few things (such as ltr).
Comment 4 Panos Astithas [:past] (away until 7/21) 2011-11-08 02:45:06 PST
Created attachment 572764 [details] [diff] [review]
Working patch

(In reply to Dave Camp (:dcamp) from comment #3)
> In bug 700036 we started moving our xhtml/html documents to
> xul-documents-with-html-as-the-default-namespace.  This helps solve a few
> things (such as ltr).

This is an interesting data point in our html vs. xul discussion. However, I propose we deal with that issue in a followup, in order to move quickly towards merging with m-c. If the debugger does indeed suffer from the same bug, I think it would be easier to get people like Ehsan to verify it using a nightly, rather than getting them to build remote-debug. Doing work in smaller chunks will also help with our growing patch queue and the merge conflicts we're encountering with Victor.
Comment 5 Panos Astithas [:past] (away until 7/21) 2011-11-08 04:02:53 PST
Created attachment 572776 [details] [diff] [review]
Working patch v2

Fixed a typo (Globel->Global).
Comment 6 Panos Astithas [:past] (away until 7/21) 2011-11-09 08:38:28 PST
(In reply to Panos Astithas [:past] from comment #4)
> Created attachment 572764 [details] [diff] [review] [diff] [details] [review]
> Working patch
> 
> (In reply to Dave Camp (:dcamp) from comment #3)
> > In bug 700036 we started moving our xhtml/html documents to
> > xul-documents-with-html-as-the-default-namespace.  This helps solve a few
> > things (such as ltr).
> 
> This is an interesting data point in our html vs. xul discussion. However, I
> propose we deal with that issue in a followup, in order to move quickly
> towards merging with m-c. 

That followup could be bug 700639.
Comment 7 Dave Camp (:dcamp) 2011-11-15 14:15:55 PST
Comment on attachment 572776 [details] [diff] [review]
Working patch v2

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

::: browser/devtools/debugger/content/debugger-overlay.js
@@ +227,5 @@
>        case "resource":
>          NetUtil.asyncFetch(url, function onAsyncFetch(stream, status) {
>            if (!Components.isSuccessCode(status)) {
> +            let view = this.getDebugger(gBrowser.selectedTab).DebuggerView;
> +            alert(view.getFormatStr("loadingError", [ url, status ]));

Can we drop this alert entirely?
Comment 8 Panos Astithas [:past] (away until 7/21) 2011-11-16 03:59:15 PST
(In reply to Dave Camp (:dcamp) from comment #7)
> Comment on attachment 572776 [details] [diff] [review] [diff] [details] [review]
> Working patch v2
> 
> Review of attachment 572776 [details] [diff] [review] [diff] [details] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/devtools/debugger/content/debugger-overlay.js
> @@ +227,5 @@
> >        case "resource":
> >          NetUtil.asyncFetch(url, function onAsyncFetch(stream, status) {
> >            if (!Components.isSuccessCode(status)) {
> > +            let view = this.getDebugger(gBrowser.selectedTab).DebuggerView;
> > +            alert(view.getFormatStr("loadingError", [ url, status ]));
> 
> Can we drop this alert entirely?

Yes, there is another one as well a few lines below. I've been meaning to take care of them after I update this part to use a fresh version from the Source Editor, after it lands. In fact I wanted to file a bug to extract these functions into a utility module. I'll convert them to Cu.reportError for now.

I'll do it as part of bug 700351, in order to avoid rebasing the rest of the patches in the queue, and the modularization patch in particular.
Comment 9 Panos Astithas [:past] (away until 7/21) 2011-11-16 04:31:47 PST
https://hg.mozilla.org/users/dcamp_campd.org/remote-debug/rev/60125dac74de

Note You need to log in before you can comment on or make changes to this bug.