The default bug view has changed. See this FAQ.

"XForms Error" dialog should have link to open JavaScript Console

RESOLVED FIXED

Status

Core Graveyard
XForms
RESOLVED FIXED
12 years ago
9 months ago

People

(Reporter: surkov, Assigned: Allan Beaufour)

Tracking

({fixed1.8, fixed1.8.0.2, fixed1.8.1})

Trunk
fixed1.8, fixed1.8.0.2, fixed1.8.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 6 obsolete attachments)

(Reporter)

Description

12 years ago
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
(Reporter)

Updated

12 years ago
Summary: "XForms Error" should can open JavaScript Console → "XForms Error" dialog should can open JavaScript Console
(Assignee)

Comment 1

12 years ago
I haven't tested this, but I had the same thought yesterday. I think it would be
ok. What do you others say?
Sounds good.

Comment 3

12 years ago
fine with me
(Reporter)

Comment 4

12 years ago
Created attachment 196976 [details]
style
(Reporter)

Comment 5

12 years ago
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).
(Assignee)

Comment 6

12 years ago
Looks ok, can you supply a patch file?
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 2000 → All
Hardware: PC → All
(Reporter)

Comment 7

12 years ago
Created attachment 197142 [details] [diff] [review]
patch
(Reporter)

Comment 8

12 years ago
Somebody wants to review the patch :)?
(Assignee)

Comment 9

12 years ago
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?
Attachment #197142 - Flags: review?(allan)
(Reporter)

Comment 10

12 years ago
(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?
(Reporter)

Comment 11

12 years ago
Description should be probably replaced by label.text-link element (see
http://xulplanet.com/references/elemref/ref_label.html#class_text-link).
(Reporter)

Comment 12

12 years ago
Created attachment 197505 [details] [diff] [review]
patch
Attachment #197142 - Attachment is obsolete: true
Attachment #197505 - Flags: review?(allan)
(Assignee)

Updated

12 years ago
Attachment #197142 - Flags: review?(allan)
(Assignee)

Comment 13

12 years ago
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.
(Reporter)

Comment 14

12 years ago
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.
Attachment #196976 - Attachment is obsolete: true
Attachment #196982 - Attachment is obsolete: true
Attachment #197505 - Attachment is obsolete: true
(Reporter)

Updated

12 years ago
Attachment #197665 - Flags: review?(allan)
(Assignee)

Updated

12 years ago
Attachment #197505 - Flags: review?(allan)
(Assignee)

Comment 15

12 years ago
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.
Attachment #197665 - Flags: review?(allan) → review+
(Reporter)

Comment 16

12 years ago
(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.
(Reporter)

Comment 17

12 years ago
Created attachment 197810 [details] [diff] [review]
patch without text-link styles
Attachment #197810 - Flags: review?(allan)
(Assignee)

Comment 18

12 years ago
Comment on attachment 197810 [details] [diff] [review]
patch without text-link styles

r=me without the bogus changes to the .css file
Attachment #197810 - Flags: review?(smaug)
Attachment #197810 - Flags: review?(allan)
Attachment #197810 - Flags: review+
Comment on attachment 197810 [details] [diff] [review]
patch without text-link styles

r=me without extra newline additions.
Attachment #197810 - Flags: review?(smaug) → review+
(Reporter)

Comment 20

12 years ago
Created attachment 197971 [details] [diff] [review]
patch
Attachment #197665 - Attachment is obsolete: true
Attachment #197810 - Attachment is obsolete: true
Attachment #197971 - Flags: review?(allan)
(Assignee)

Comment 21

12 years ago
Checked in to trunk
Assignee: aaronr → allan
Whiteboard: xf-to-branch
(Assignee)

Updated

12 years ago
Status: NEW → ASSIGNED
(Assignee)

Updated

12 years ago
Attachment #197971 - Flags: review?(allan)

Comment 22

12 years ago
checked into branch 20051004
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Whiteboard: xf-to-branch

Updated

12 years ago
Summary: "XForms Error" dialog should can open JavaScript Console → "XForms Error" dialog should have link to open JavaScript Console

Updated

12 years ago
Keywords: fixed1.8

Comment 23

12 years ago
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

12 years ago
Oh, and as the dialog opens *before* logging the error to the console, opening the console is not much help in any case.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Comment 25

12 years ago
(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?
(Reporter)

Comment 26

12 years ago
(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

12 years ago
(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);
}
(Reporter)

Comment 28

12 years ago
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?
Attachment #203227 - Flags: review?(aaronr)

Comment 29

12 years ago
(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.

Updated

12 years ago
Attachment #203227 - Flags: review?(aaronr) → review+
(Reporter)

Comment 30

12 years ago
(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.
(Assignee)

Updated

12 years ago
Attachment #203227 - Flags: review+
(Assignee)

Comment 31

12 years ago
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
Attachment #203227 - Attachment description: patch → patch that fix open()

Comment 32

12 years ago
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

11 years ago
can we close this one now since we are using bug 316609 for the a11y stuff?
(Assignee)

Comment 34

11 years ago
(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.
Status: REOPENED → RESOLVED
Last Resolved: 12 years ago11 years ago
Keywords: fixed1.8.0.2, fixed1.8.1
Resolution: --- → FIXED
(Reporter)

Comment 35

11 years ago
Should Neil's comments be fixed in bug 316609 too?
(Assignee)

Comment 36

11 years ago
(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.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.