Cross-Synchronize Toolkit and XPFE <consoleBindings.xml>

RESOLVED WONTFIX

Status

defect
RESOLVED WONTFIX
13 years ago
9 years ago

People

(Reporter: sgautherie, Assigned: sgautherie)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

2 from XPFE: bug 288544 bug 290429
3 from Toolkit: bug 301398 bug 302402 bug 306223
1 more from XPFE: bug 179268
Depends on: 179268
Depends on: 289927
Depends on: 322169
1 more from Toolkit: bug 289927 ... Done there.
No longer depends on: 289927
Bug 301398 port,
as it was checked in [focus border color is '-moz-DialogText' instead of 'black' initialy];
This '-Bw' patch is for review only.

[Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8.1b1) Gecko/20060717 SeaMonkey/1.1a] (nightly) (W98SE)

I did a minimal test with this build in Classic theme, which seems to be working well...
Assignee: js-console → gautheri
Status: NEW → ASSIGNED
Attachment #229934 - Flags: review?(neil)
Comment on attachment 229934 [details] [diff] [review]
(Av1-XPFE-Bw) Bug 301398 port

The CSS changes look good although I haven't tried them yet.

>-      <xul:label class="text-link" xbl:inherits="value=url" crop="right"/>
>+      <xul:label class="text-link" xbl:inherits="href,value=href" crop="right"/>
I don't understand the attribute name change here - this isn't a "real" text link because we open the view source window, so it doesn't need an href.

>-label.text-link {
>+label.text-link, label[onclick] {
I don't think this change is correct at all.
CC'ing Aaron, who, I hope, could answer your questions as Toolkit patch author,
because I obviously can't.
Bug 306223 was ported to SeaMonkey there.
No longer depends on: 306223
Can you do a patch just with the outline CSS changes? Thanks.
(AFAIK outline: invert; doesn't work properly in Cairo builds.)
With:
Only the 3 <formatting.css> files !?
Only the 3 |.text-link:focus| rules changes !?
(In reply to comment #4)
> >-      <xul:label class="text-link" xbl:inherits="value=url" crop="right"/>
> >+      <xul:label class="text-link" xbl:inherits="href,value=href" crop="right"/>
> I don't understand the attribute name change here - this isn't a "real" text
> link because we open the view source window, so it doesn't need an href.
We use the href in nsXULTextAccessible.cpp in 2 places. We use it to expose the accessible value of the XUL link, which should be the URL. We also use it to know which element should receive a click when the link is supposed to be activated (some XUL links are unfortunately created with a pair of elements, a parent and a child).

> >-label.text-link {
> >+label.text-link, label[onclick] {
> I don't think this change is correct at all.
This change makes it so that when a XUL author places an onclick on a XUL label, it will still be accessible. The appearance isn't being changed here, but the way it is exposed to accessibility is. It should be exposed as a link or a button -- something actionable, and needs to be tabbable. If we don't do this we keep ending up with inaccessible XUL clickable labels.

OK, but then this patch is just missing the change to consoleBindings.xml that makes it only open the view source window and not the original URL.
Trunk is now using the toolkit Error Console. Do we need this for branch?
(In reply to comment #1)
> 1 more from XPFE: bug 179268

Neil,
Should "I" port your <consoleBindings.xml> patch there to </toolkit/> ?
(In reply to comment #12)
> (In reply to comment #1)
> > 1 more from XPFE: bug 179268
> Neil,
> Should "I" port your <consoleBindings.xml> patch there to </toolkit/> ?
If they want it, then feel free to port it.
Product: Core → SeaMonkey
Blocks: 386899
Comment on attachment 229934 [details] [diff] [review]
(Av1-XPFE-Bw) Bug 301398 port

patch expired/bitrotted
Attachment #229934 - Flags: review?(neil)
(In reply to comment #11)
> Trunk is now using the toolkit Error Console. Do we need this for branch?

Yes this seems done.. EXPIRED; reopen if I'm wrong.(
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
Product: SeaMonkey → Core Graveyard
You need to log in before you can comment on or make changes to this bug.