Last Comment Bug 309246 - "XForms Error" dialog should have link to open JavaScript Console
: "XForms Error" dialog should have link to open JavaScript Console
Status: RESOLVED FIXED
: fixed1.8, fixed1.8.0.2, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Allan Beaufour
: Stephen Pride
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2005-09-19 21:09 PDT by alexander :surkov
Modified: 2016-07-15 14:46 PDT (History)
4 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
style (148 bytes, text/css)
2005-09-21 18:12 PDT, alexander :surkov
no flags Details
dialog (2.67 KB, application/vnd.mozilla.xul+xml)
2005-09-21 18:21 PDT, alexander :surkov
no flags Details
patch (950 bytes, patch)
2005-09-22 21:50 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
patch (1.49 KB, patch)
2005-09-26 20:39 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
patch (1.85 KB, patch)
2005-09-27 22:35 PDT, alexander :surkov
allan: review+
Details | Diff | Splinter Review
patch without text-link styles (1.52 KB, patch)
2005-09-28 22:41 PDT, alexander :surkov
allan: review+
bugs: review+
Details | Diff | Splinter Review
patch (1.14 KB, patch)
2005-09-30 00:32 PDT, alexander :surkov
no flags Details | Diff | Splinter Review
patch that fix open() (1.73 KB, patch)
2005-11-15 23:55 PST, alexander :surkov
aaronr: review+
allan: review+
Details | Diff | Splinter Review

Description alexander :surkov 2005-09-19 21:09:26 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8b4) Gecko/20050908 Firefox/1.4

You can do something like this:

bindingex.xul

<description class="descr2" 
  onclick="open('chrome://global/content/console.xul', 'global:console', 
'menubar=no,resizable=yes')">
  &xforms.bindingdialog.description2;
</description>

bindingex.css

.descr2 { 
  font-size: 0.8em;
  color: blue;
} 

.descr2:hover{
	cursor: pointer;
}

Reproducible: Always
Comment 1 Allan Beaufour 2005-09-21 03:41:07 PDT
I haven't tested this, but I had the same thought yesterday. I think it would be
ok. What do you others say?
Comment 2 Olli Pettay [:smaug] 2005-09-21 04:27:19 PDT
Sounds good.
Comment 3 aaronr 2005-09-21 09:59:23 PDT
fine with me
Comment 4 alexander :surkov 2005-09-21 18:12:44 PDT
Created attachment 196976 [details]
style
Comment 5 alexander :surkov 2005-09-21 18:21:35 PDT
Created attachment 196982 [details]
dialog

Please test it and correct styles and content as right for you.

I'm using open() method instead of openDialog() because it seems to me "XForms
Error" dialog is opened modal (bug 256555).
Comment 6 Allan Beaufour 2005-09-22 06:09:36 PDT
Looks ok, can you supply a patch file?
Comment 7 alexander :surkov 2005-09-22 21:50:48 PDT
Created attachment 197142 [details] [diff] [review]
patch
Comment 8 alexander :surkov 2005-09-26 03:23:25 PDT
Somebody wants to review the patch :)?
Comment 9 Allan Beaufour 2005-09-26 03:54:36 PDT
Comment on attachment 197142 [details] [diff] [review]
patch

>diff -uNr content.orig/bindingex.css content/bindingex.css
>--- content.orig/bindingex.css	2005-09-23 13:34:00.000000000 +0900
>+++ content/bindingex.css	2005-09-23 13:47:30.000000000 +0900

The file has Windows line-endings, you need to convert it to Unix-style.

>@@ -4,4 +4,9 @@
> 
> .descr2 { 
>   font-size: 0.8em;
>+  color: blue;

I do not know whether there is a "link"-class we could use instead, to use the
generic styling of links?

>+}
>+
>+.descr2:hover{

Add space before {

>+	cursor: pointer;

Do not use tabs.

> }
>\ В конце файла нет новой строки

What's this?
Comment 10 alexander :surkov 2005-09-26 04:15:17 PDT
(In reply to comment #9)
> I do not know whether there is a "link"-class we could use instead, to use the
> generic styling of links?
> 

There is link styles at chrome://communicator/skin/formatting.css. I think
xforms shouldn't depend on communicator styles. Threafore thease styles can be
included at bindings.css directly. Is it satisfactory solution?
Comment 11 alexander :surkov 2005-09-26 04:27:44 PDT
Description should be probably replaced by label.text-link element (see
http://xulplanet.com/references/elemref/ref_label.html#class_text-link).
Comment 12 alexander :surkov 2005-09-26 20:39:50 PDT
Created attachment 197505 [details] [diff] [review]
patch
Comment 13 Allan Beaufour 2005-09-27 06:00:35 PDT
Comment on attachment 197505 [details] [diff] [review]
patch

You still use Windows line-endings.

>diff -uNr mozilla.orig/extensions/xforms/resources/content/bindingex.css mozilla/extensions/xforms/resources/content/bindingex.css
>--- mozilla.orig/extensions/xforms/resources/content/bindingex.css	2005-09-27 12:35:58.000000000 +0900
>+++ mozilla/extensions/xforms/resources/content/bindingex.css	2005-09-27 12:35:13.000000000 +0900

You do not need the styles. They are included automatically, just set the
"text-link" class on the label. That is enough. That way, a user can also style
text-link as he wants.

>diff -uNr mozilla.orig/extensions/xforms/resources/content/bindingex.xul mozilla/extensions/xforms/resources/content/bindingex.xul
>--- mozilla.orig/extensions/xforms/resources/content/bindingex.xul	2005-09-27 12:06:09.000000000 +0900
>+++ mozilla/extensions/xforms/resources/content/bindingex.xul	2005-09-27 12:35:50.000000000 +0900
>@@ -53,9 +53,9 @@
>       <description class="header descr1">
>         &xforms.bindingdialog.description1;
>       </description>
>-      <description class="descr2">
>+      <label class="descr2 text-link" onclick="open('chrome://global/content/console.xul', 'global:console', 'menubar=no,resizable=yes')">
>         &xforms.bindingdialog.description2;
>-      </description>
>+      </label>
>     </vbox>
>   </hbox>
> </dialog>

Fine. Only problem is that the label gets focus by default... I wonder whether
you can instruct the dialog to give the button focus as default? With that
fixed, I'm fine with this patch.
Comment 14 alexander :surkov 2005-09-27 22:35:51 PDT
Created attachment 197665 [details] [diff] [review]
patch

I hope now I use Unix line-endings :)

> You do not need the styles. They are included automatically, just set the
> "text-link" class on the label. That is enough. That way, a user can also
style
> text-link as he wants.

I don't agree. They will not be included automatically if I run application
with -chrome command line argument. In that case communicator styles aren't
attached. In any case user can style text-link.
Comment 15 Allan Beaufour 2005-09-28 04:54:11 PDT
Comment on attachment 197665 [details] [diff] [review]
patch

Good focus fix.

The -chrome issue is a special case in my view. We should look like any other
"thing" in Firefox, thus follow the standard styles.

Remove the css, and you have a r=me.
Comment 16 alexander :surkov 2005-09-28 22:27:52 PDT
(In reply to comment #15)
> The -chrome issue is a special case in my view. We should look like any other
> "thing" in Firefox, thus follow the standard styles.
> 
> Remove the css, and you have a r=me.
> 

As I understand you wants to see xforms as thing for browser. I think it's
ignominious with respect to people who used xforms in application. Application
is situated in own window (not in browser window) and thus link will not be
looked as link. I would propose to include browser styles into xforms dialog but
sometimes I guess browser package can be absent.

Truth to tell I think link styles should be included in global styles and
threafore I should remove them. I posted a bug 310403.
Comment 17 alexander :surkov 2005-09-28 22:41:51 PDT
Created attachment 197810 [details] [diff] [review]
patch without text-link styles
Comment 18 Allan Beaufour 2005-09-29 04:36:36 PDT
Comment on attachment 197810 [details] [diff] [review]
patch without text-link styles

r=me without the bogus changes to the .css file
Comment 19 Olli Pettay [:smaug] 2005-09-29 07:23:05 PDT
Comment on attachment 197810 [details] [diff] [review]
patch without text-link styles

r=me without extra newline additions.
Comment 20 alexander :surkov 2005-09-30 00:32:17 PDT
Created attachment 197971 [details] [diff] [review]
patch
Comment 21 Allan Beaufour 2005-09-30 02:00:38 PDT
Checked in to trunk
Comment 22 aaronr 2005-10-06 17:34:33 PDT
checked into branch 20051004
Comment 23 neil@parkwaycc.co.uk 2005-11-09 08:04:50 PST
Comment on attachment 197971 [details] [diff] [review]
patch

>+        onload="document.getElementById('XFormsBindingDialog').getButton('accept').focus();">
This could use document.documentElement.

>+      <label class="descr2 text-link" onclick="open('chrome://global/content/console.xul', 'global:console', 'menubar=no,resizable=yes')">
This is not the right way to open the JS console. Not only does it not solve the modality issue it actually opens a browser window and loads the JS console inside it. Note that the global:console is a red herring because it's not a windowtype.
Comment 24 neil@parkwaycc.co.uk 2005-11-09 08:29:45 PST
Oh, and as the dialog opens *before* logging the error to the console, opening the console is not much help in any case.
Comment 25 alexander :surkov 2005-11-09 23:38:52 PST
(In reply to comment #23)
> (From update of attachment 197971 [details] [diff] [review] [edit])
> >+        onload="document.getElementById('XFormsBindingDialog').getButton('accept').focus();">
> This could use document.documentElement.
> 

Ok

> >+      <label class="descr2 text-link" onclick="open('chrome://global/content/console.xul', 'global:console', 'menubar=no,resizable=yes')">
> This is not the right way to open the JS console. Not only does it not solve
> the modality issue it actually opens a browser window and loads the JS console
> inside it. Note that the global:console is a red herring because it's not a
> windowtype.
> 

Why doesn't open() method solve the modality issue and why is open() method so bad for js console opening? What the way should I choose to open js console?
Comment 26 alexander :surkov 2005-11-09 23:41:21 PST
(In reply to comment #24)
> Oh, and as the dialog opens *before* logging the error to the console, opening
> the console is not much help in any case.
> 

I guess some time before it worked. Does anybody know what was happen?
Comment 27 neil@parkwaycc.co.uk 2005-11-13 13:57:50 PST
(In reply to comment #25)
>What is the way should I choose to open js console?
function toJSConsole() {
  var js = Components.classes["@mozilla.org/appshell/window-mediator;1"]
                     .getService(Components.interfaces.nsIWindowMediator)
                     .getMostRecentWindow('global:console');
  if (js)
    js.focus();
  else
    Components.classes["@mozilla.org/embedcomp/window-watcher;1"]
              .getService(Components.interfaces.nsIWindowWatcher)
              .openWindow(null, "chrome://global/content/console.xul",
                          "", "all,dialog=no", null);
}
Comment 28 alexander :surkov 2005-11-15 23:55:40 PST
Created attachment 203227 [details] [diff] [review]
patch that fix open()

Patch with Neil comments.

Aaron, please review the patch and say described problems from bug 316609 still exist?
Comment 29 aaronr 2005-11-17 14:00:56 PST
(In reply to comment #28)
> Created an attachment (id=203227) [edit]
> patch
> 
> Patch with Neil comments.
> 
> Aaron, please review the patch and say described problems from bug 316609 still
> exist?
> 

This patch fixes the problem where tabbing to the label and pressing 'enter' doesn't work on Firefox.  We still need some visual indicator that it is something that can be tabbed too or clicked on.  Like in Seamonkey, the color of the label is black and it doesn't get any kind of focus indicator when you tab to it.
Comment 30 alexander :surkov 2005-11-17 19:04:43 PST
(In reply to comment #29)

> We still need some visual indicator that it is
> something that can be tabbed too or clicked on.  Like in Seamonkey, the color
> of the label is black and it doesn't get any kind of focus indicator when you
> tab to it.
> 

Do you see a visual problem in seamonkey only? If it is true then problem will go away when bug 310403 will be fixed.
Comment 31 Allan Beaufour 2005-11-17 22:24:39 PST
Comment on attachment 203227 [details] [diff] [review]
patch that fix open()

I've checked this in to trunk, the visual a11y stuff goes on bug 316609
Comment 32 neil@parkwaycc.co.uk 2005-11-18 16:29:32 PST
Comment on attachment 203227 [details] [diff] [review]
patch that fix open()

>+    function toJSConsole() {
...
>+      }
Nit: } doesn't line up with function

>+      <label class="descr2 text-link" onclick="toJSConsole()">
While I'm nitpicking, it's neater to write "toJSConsole();"
Comment 33 aaronr 2006-02-07 23:47:30 PST
can we close this one now since we are using bug 316609 for the a11y stuff?
Comment 34 Allan Beaufour 2006-02-07 23:58:54 PST
(In reply to comment #33)
> can we close this one now since we are using bug 316609 for the a11y stuff?

Well, the last fix was only checked in on trunk. I just checked it in on 1_8_0 and 1_8. Closing it.
Comment 35 alexander :surkov 2006-02-08 00:17:20 PST
Should Neil's comments be fixed in bug 316609 too?
Comment 36 Allan Beaufour 2006-02-08 00:35:13 PST
(In reply to comment #35)
> Should Neil's comments be fixed in bug 316609 too?

It's nits, but if you remember to fix it in bug 316609 that would be nice.

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