autocomplete handler inconsistencies in the toolkits

RESOLVED FIXED

Status

RESOLVED FIXED
14 years ago
12 years ago

People

(Reporter: p_ch, Assigned: ajschult784)

Tracking

({fixed-seamonkey1.1, fixed1.8.1.1})

Trunk
x86
All
fixed-seamonkey1.1, fixed1.8.1.1

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

14 years ago
Seamonkey toolkit autocomplete widget handlers are: ontextcommand and
ontextrevert. The new toolkit ones are ontextentered and ontextreverted.
As a result, hitting return in the dom inspector url bar does not load the
document in firefox.

The last ones are more consistent with other handlers like onpopupshowing,
onpopupshown and I propose to deprecate ontextcommand and ontextrevert.

The patch I will attach does not get rid of them, but the handlers ontextentered
and ontextreverted would have the precedence.
(Reporter)

Comment 1

14 years ago
Created attachment 156230 [details] [diff] [review]
patch seamonkey autocomplete.xml + inspector

If module owners are ok and even if I know thunderbird does not use the new
toolkit autocomplete widget, I am ready to change all the occurence of
ontextcommand and ontextrevert to the new ones and modify also mscott's version
of autocomplete.xml.
This patch has been tested on mozilla 1.7.2
Assignee: nobody → p_ch
Status: NEW → ASSIGNED
(Reporter)

Updated

14 years ago
Attachment #156230 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #156230 - Flags: review?(neil.parkwaycc.co.uk)
(Reporter)

Updated

14 years ago
Attachment #156230 - Flags: review?(neil.parkwaycc.co.uk) → review?(caillon)
Comment on attachment 156230 [details] [diff] [review]
patch seamonkey autocomplete.xml + inspector

Unforking is good.
Attachment #156230 - Flags: review?(caillon) → review+
Product: Core → Mozilla Application Suite

Comment 3

13 years ago
Comment on attachment 156230 [details] [diff] [review]
patch seamonkey autocomplete.xml + inspector

sr=me if you change all the occurrances not just inspector but remove the
fallback code as extensions can just provide both sets of attributes if
necessary.
Attachment #156230 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview+
(Assignee)

Comment 4

13 years ago
Created attachment 208058 [details] [diff] [review]
fix the rest of xpfe and mail
Assignee: p_ch → ajschult
Attachment #156230 - Attachment is obsolete: true
Attachment #208058 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #208058 - Flags: review?(neil.parkwaycc.co.uk)

Comment 5

13 years ago
Comment on attachment 208058 [details] [diff] [review]
fix the rest of xpfe and mail

>-                 ontextcommand="return handleURLBarCommand(eventParam, domEvent);"
>+                 ontextentered="return handleURLBarCommand(eventParam, domEvent);"
>                  ontextrevert="return handleURLBarRevert();"
Missed one :-P

There are a bunch of fields with related names in autocomplete.xml that look unused.

I still don't understand why you want the fallback code, anyone writing a dual suite/toolkit compatible extension will already have had to provide both sets of attribtues.
(Assignee)

Comment 6

13 years ago
Created attachment 208170 [details] [diff] [review]
patch v3

fixup ontextrevert, remove backwards-compat and nuke obsolete fields

I'd guess the backwards-compat is so that current suite-only extensions don't break... not that I'd guess there are many.  :(
Attachment #208058 - Attachment is obsolete: true
Attachment #208170 - Flags: superreview?
Attachment #208170 - Flags: review?(neil.parkwaycc.co.uk)
Attachment #208058 - Flags: superreview?(neil.parkwaycc.co.uk)
Attachment #208058 - Flags: review?(neil.parkwaycc.co.uk)
(Assignee)

Updated

13 years ago
Attachment #208170 - Flags: superreview? → superreview?(neil.parkwaycc.co.uk)

Comment 7

13 years ago
Comment on attachment 208170 [details] [diff] [review]
patch v3

I guess you can also kill <field name="onerrorcommand">.

Probably a good idea to get mail sign-off on this too.
Attachment #208170 - Flags: review?(neil.parkwaycc.co.uk) → review+
(Assignee)

Updated

13 years ago
Attachment #208170 - Flags: superreview?(neil.parkwaycc.co.uk) → superreview?(mscott)

Updated

13 years ago
Attachment #208170 - Flags: superreview?(mscott) → superreview+
(Assignee)

Comment 8

13 years ago
FIXED
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED

Comment 9

12 years ago
Comment on attachment 208170 [details] [diff] [review]
patch v3

Neil suggested approval-seamonkey1.1 was the right way to get this into TB2. Please.
Attachment #208170 - Flags: approval-seamonkey1.1?

Updated

12 years ago
Blocks: 360648

Comment 10

12 years ago
Comment on attachment 208170 [details] [diff] [review]
patch v3

This sounds to me like it touches more than just SeaMonkey and needs general 1.8.1.1 approval instead.
Attachment #208170 - Flags: approval1.8.1.1?
Comment on attachment 208170 [details] [diff] [review]
patch v3

approved for 1.8 branch, a=dveditz for drivers
Attachment #208170 - Flags: approval1.8.1.1? → approval1.8.1.1+
Comment on attachment 208170 [details] [diff] [review]
patch v3

a=me for sm1.1
Attachment #208170 - Flags: approval-seamonkey1.1? → approval-seamonkey1.1+
(Assignee)

Updated

12 years ago
Keywords: fixed-seamonkey1.1, fixed1.8.1.1
(Assignee)

Comment 13

12 years ago
(I landed on the branch without Neil's suggestion from comment 7, see bug 362163)
You need to log in before you can comment on or make changes to this bug.