Closed Bug 727954 Opened 8 years ago Closed 2 years ago

Port |Bug 717772 - Delay autocomplete of pasted value| to SeaMonkey

Categories

(SeaMonkey :: Autocomplete, defect, P2, major)

Tracking

(firefox12 verified, seamonkey2.9- verified)

RESOLVED FIXED
seamonkey2.10
Tracking Status
firefox12 --- verified
seamonkey2.9 - verified

People

(Reporter: sgautherie, Assigned: neil)

References

(Depends on 1 open bug, Blocks 1 open bug, )

Details

(Whiteboard: [perma-orange])

Attachments

(2 files)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329215161.1329219819.16122.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/02/14 02:26:01
{
19557 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_autocomplete_delayOnPaste.xul | Test timed out.
}

Need to port
http://hg.mozilla.org/mozilla-central/diff/19a5e75b8ed8/toolkit/content/widgets/autocomplete.xml
Neil, do you confirm we need to port that change to the XPFE widget?
Or should this test be forced to use the Toolkit widget? (If not already.)
Or ...
(In reply to Serge Gautherie from comment #1)
> Neil, do you confirm we need to port that change to the XPFE widget?
Yeah, I guess so...

> Or should this test be forced to use the Toolkit widget? (If not already.)
Nontrivial, I'm afraid, until LDAP autocomplete is sorted out...
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #601712 - Flags: review?(sgautherie.bz)
(In reply to Serge Gautherie (:sgautherie) from comment #0)
> http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329215161.1329219819.
> 16122.gz
> WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/02/14
> 02:26:01

{
19554 INFO TEST-START | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_autocomplete_delayOnPaste.xul
### ERROR - unable to create search "simple".
19555 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_autocomplete_delayOnPaste.xul | Clipboard has the correct value
19556 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_autocomplete_delayOnPaste.xul | Value should not be autocompleted immediately - res should equal res
}

[Mozilla/5.0 (Windows NT 5.0; rv:13.0a1) Gecko/20120201 Firefox/13.0a1 SeaMonkey/2.10a1] (nightly, 2012-02-01-00-30-09-comm-central-trunk)

I tested this patch in that build, but it makes no difference:
{
0 INFO SimpleTest START
1 INFO TEST-START | chrome://mochitests/content/chrome/toolkit/content/tests/chr
ome/test_autocomplete_delayOnPaste.xul
search "simple" not found - skipping.
2 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/content/tests/chro
me/test_autocomplete_delayOnPaste.xul | Clipboard has the correct value
3 INFO TEST-PASS | chrome://mochitests/content/chrome/toolkit/content/tests/chro
me/test_autocomplete_delayOnPaste.xul | Value should not be autocompleted immedi
ately - res should equal res
}

Is |search "simple" not found - skipping.| an issue (to fix (first))?
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

(I wouldn't be a reviewer for this code.)
Attachment #601712 - Flags: review?(sgautherie.bz)
(In reply to Serge Gautherie from comment #4)
> Is |search "simple" not found - skipping.| an issue (to fix (first))?
Yes, unfortunately XPFE autocomplete can't easily defer its initialisation so the workaround is to move autoCompleteSimple.registerFactory() to the top level rather than waiting for the load event.
Fixes comment 4 timeout/issue (only), per comment 6.

Also, unrelated but safer:
*Execute waitForExplicitFinish() before load event.
*Get $("autocomplete") after load event.
 *Ftr, it also breaks XPFE if executed before registration.
Attachment #601964 - Flags: review?(mak77)
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

After applying patch Bv1, this fixes what that test is really about ;-)
Attachment #601712 - Flags: feedback+
Comment on attachment 601964 [details] [diff] [review]
(Bv1) test_autocomplete_delayOnPaste.xul: Support XPFE AutoComplete widget too
[Checked in: Comment 12]

>-let gAutoComplete = $("autocomplete");
Oh yes, this is always a bad idea in XUL.
Attachment #601712 - Flags: review?(iann_bugzilla)
Attachment #601712 - Flags: review?(iann_bugzilla) → review+
Pushed changeset ace2338e90df to mozilla-central.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Attachment #601964 - Flags: review?(mak77) → review+
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

[Approval Request Comment]
Regression caused by (bug #): (Port Firefox enhancement to SeaMonkey.)
User impact if declined: (SeaMonkey security related.)
Testing completed (on m-c, etc.): Comment 10.
Risk to taking this patch (and alternatives if risky): None (for FF), NPOfirefoxB.
String changes made by this patch: None.
Attachment #601712 - Flags: approval-mozilla-aurora?
Comment on attachment 601964 [details] [diff] [review]
(Bv1) test_autocomplete_delayOnPaste.xul: Support XPFE AutoComplete widget too
[Checked in: Comment 12]

https://hg.mozilla.org/mozilla-central/rev/5309b25db6de


[Approval Request Comment]
Regression caused by (bug #): Bug 717772.
User impact if declined: None, but perma-orange (timeout) on SeaMonkey.
Testing completed (on m-c, etc.): This comment.
Risk to taking this patch (and alternatives if risky): None, test-only.
String changes made by this patch: None.
Attachment #601964 - Attachment description: (Bv1) test_autocomplete_delayOnPaste.xul: Support XPFE AutoComplete widget too → (Bv1) test_autocomplete_delayOnPaste.xul: Support XPFE AutoComplete widget too [Checked in: Comment 12]
Attachment #601964 - Flags: approval-mozilla-aurora?
Attachment #601712 - Attachment description: Proposed patch → Proposed patch [Checked in: Comment 10]
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331027114.1331031690.5242.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2012/03/06 01:45:14
{
19476 INFO TEST-END | chrome://mochitests/content/chrome/toolkit/content/tests/chrome/test_autocomplete_delayOnPaste.xul | finished in 2457ms
}

V.Fixed
Status: RESOLVED → VERIFIED
Flags: in-testsuite+
Depends on: 733396
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

[Approval Request Comment]
See comment 12.
Attachment #601712 - Flags: approval-mozilla-aurora? → approval-comm-aurora?
Attachment #601712 - Flags: approval-comm-aurora? → approval-comm-aurora+
Comment on attachment 601964 [details] [diff] [review]
(Bv1) test_autocomplete_delayOnPaste.xul: Support XPFE AutoComplete widget too
[Checked in: Comment 12]

approving, test-only change.
Attachment #601964 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Keywords: checkin-needed
Whiteboard: [perma-orange] → [c-n: 5309b25db6de to c-a (and ace2338e90df to m-a)] [perma-orange]
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

This is an m-c patch, thus clearing comm approval flag. Please re-request accordingly. [AFAICS you had that initially, then why in the world did you change it?]

Since I don't know whether the parts may be checked in individually I'll also clear the checkin-needed flag until that is clarified.
Attachment #601712 - Flags: approval-comm-aurora+
Keywords: checkin-needed
Whiteboard: [c-n: 5309b25db6de to c-a (and ace2338e90df to m-a)] [perma-orange] → [perma-orange]
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #16)

> This is an m-c patch, thus clearing comm approval flag. Please re-request
> accordingly. [AFAICS you had that initially, then why in the world did you
> change it?]

Because it touches an Xpfe part that is (now) used by SeaMonkey only, though it still lives in m-c:
it occurred to me that c-* approval should be enough nowadays.

Yet, I'll request approval(s) again, and ask for a confirmation on both side.

> Since I don't know whether the parts may be checked in individually

The two parts are separate issues.
Keywords: checkin-needed
Whiteboard: [perma-orange] → [c-n: 5309b25db6de to c-a] [perma-orange]
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

[Approval Request Comment]
Please see comment 12 and comment 17:
I would also like to get an explicit "future rs+" on mozilla-* side for such Xpfe code which is (now) SeaMonkey specific but has not (yet) moved to comm-*. Thanks.
Attachment #601712 - Flags: approval-mozilla-aurora?
Attachment #601712 - Flags: approval-comm-aurora?
Comment on attachment 601712 [details] [diff] [review]
Proposed patch
[Checked in: Comment 10]

its NPOTB for Firefox/B2G/Mobile, thus it can inherently land there, but as it is POTB for SeaMonkey, I'd like to still have us approve it until/unless it moves.

Its kinda a shady area of gray, and I'm sorry I didn't explicitly say why I was approving it myself when I did so. I don't think a rs+ is worthwhile, but I also don't think that we need explicit Mozilla-Driver approvals for the suite-only code here.

(We still, of course, have to follow tree rules elsewhere for these landings, like closed for maintenance, etc.)

I would mark a-m-a+ if I could, since I cant' its a-c-a+
Attachment #601712 - Flags: approval-mozilla-aurora?
Attachment #601712 - Flags: approval-comm-aurora?
Attachment #601712 - Flags: approval-comm-aurora+
(In reply to Justin Wood (:Callek) from comment #19)
> its NPOTB for Firefox/B2G/Mobile, thus it can inherently land there, but as
> it is POTB for SeaMonkey, I'd like to still have us approve it until/unless
> it moves.

Then, fwiw, I think we should record this unusual case, for example, on
http://www.seamonkey-project.org/dev/project-areas
 add an Xpfe line in "Cross-suite components"
and
http://www.seamonkey-project.org/dev/review-and-flags
 add a comment in "approval-comm-XXX"
Whiteboard: [c-n: 5309b25db6de to c-a] [perma-orange] → [c-n: 5309b25db6de to c-a (and ace2338e90df to m-a)] [perma-orange]
Whiteboard: [c-n: 5309b25db6de to c-a (and ace2338e90df to m-a)] [perma-orange] → [c-n: to m-a] [perma-orange]
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1331290603.1331295719.18112.gz
OS X 10.6 comm-aurora debug test mochitest-other on 2012/03/09 02:56:43

firefox12 and seamonkey2.9: verified.
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/8a1a95f644d4
Delay autocomplete of pasted value r=IanNDONTBUILD
Status: VERIFIED → RESOLVED
Closed: 8 years ago2 years ago
You need to log in before you can comment on or make changes to this bug.