Closed
Bug 441526
Opened 16 years ago
Closed 16 years ago
Implement highlightNonMatches in toolkit autocomplete
Categories
(Toolkit :: Autocomplete, defect, P2)
Toolkit
Autocomplete
Tracking
()
RESOLVED
FIXED
mozilla1.9.1a2
People
(Reporter: standard8, Assigned: standard8)
References
Details
Attachments
(3 files, 4 obsolete files)
(deleted),
patch
|
enndeakin
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Bienvenu
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
Toolkit autocomplete should implement xpfe's highlightNonMatches attribute. This is used in the autocomplete widget to bring the user's attention to the fact that an email address hasn't been matched in the address book.
TB & SM wish to keep this functionality as they move from xpfe to toolkit.
Assignee | ||
Comment 1•16 years ago
|
||
This is how I'm proposing to implement highlightNonMatches for toolkit. This patch can only be applied after the patch for bug 441530 is applied.
Includes test case extension, I wasn't sure if it was necessary to check the style as well, but I thought it wouldn't hurt to.
Assignee | ||
Comment 2•16 years ago
|
||
I'm proposing that we sync the xpfe with the toolkit changes.
The only thing this patch looses is highlighting when autocomplete to my domain is on. I'm not too worried about this, considering that when it is on at the moment, there is inconsistent colouring, and when a non-matching option is entered it currently doesn't have any highlighting.
Assignee | ||
Comment 3•16 years ago
|
||
Comment on attachment 326662 [details] [diff] [review]
Proposed sync - xpfe
I'll get additional reviews once I know you're happy with it Neil.
Attachment #326662 -
Flags: superreview?(neil)
Attachment #326662 -
Flags: review?(neil)
Comment 4•16 years ago
|
||
Comment on attachment 326662 [details] [diff] [review]
Proposed sync - xpfe
>diff --git a/xpfe/components/autocomplete/resources/content/autocomplete.css b/xpfe/components/autocomplete/resources/content/autocomplete.css
>--- a/xpfe/components/autocomplete/resources/content/autocomplete.css
>+++ b/xpfe/components/autocomplete/resources/content/autocomplete.css
>@@ -1,3 +1,7 @@
>+
>+textbox[type="autocomplete"][nomatch="true"][highlightnonmatches="true"]) {
>+ color: red;
>+}
Three issues here:
1. Extra )
2. type="autocomplete" is probably superfluous here.
2. I don't think it's correct to move the style rule into content.
By the way, I've worked out why toolkit sets nomatch on the popup.
The attribute was (and still is in suite) used by the search engine entry, to remove the top border when there are no autocomplete items. Because xpfe uses different XBL the attribute is automatically propagated to the popup. Presumably there was a time after toolkit rewrote its autocomplete but before it switched to a separate search bar when it needed to have the attribute set on the popup.
Attachment #326662 -
Flags: superreview?(neil)
Attachment #326662 -
Flags: review?(neil)
Attachment #326662 -
Flags: review-
Assignee | ||
Updated•16 years ago
|
Priority: -- → P2
Assignee | ||
Comment 5•16 years ago
|
||
Revised fix. I realised what my mistake was last time - forgetting that SeaMonkey classic and Thunderbird themes also use toolkit. So I need to put this in as one patch rather than two.
This will provide us with the correct settings and a unit test.
I'll get toolkit review once I know we're happy with the xpfe parts.
Note: you'll need to apply the patch on bug 441530 if testing on cvs (though I'm not going to check that or this one in on cvs), and you may or may not need to apply it if testing on hg.
Attachment #326661 -
Attachment is obsolete: true
Attachment #326662 -
Attachment is obsolete: true
Attachment #329865 -
Flags: superreview?(neil)
Attachment #329865 -
Flags: review?(neil)
Comment 6•16 years ago
|
||
Comment on attachment 329865 [details] [diff] [review]
The fix
> textbox[type="autocomplete"] {
> cursor: default !important;
>+}
>+
>+textbox[type="autocomplete"][nomatch="true"][highlightnonmatches="true"] {
>+ color: red;
> }
As in the toolkit themes, the [type="autocomplete"] should be unnecessary.
[In fact, I'm not actually sure why we have that cursor rule at all.]
> this.mNeedToComplete = false;
>- this.removeAttribute("noMatchesFound");
> this.clearTimer();
> this.currentSearchString = str;
> this.resultsPopup.clearSelection();
>- this.removeAttribute("noMatchesFound");
I managed to get into a state whereby I couldn't get rid of the red highlight but this may be an existing bug exposed by testing with invalid LDAP settings.
Attachment #329865 -
Flags: superreview?(neil)
Attachment #329865 -
Flags: superreview+
Attachment #329865 -
Flags: review?(neil)
Attachment #329865 -
Flags: review+
Assignee | ||
Comment 7•16 years ago
|
||
Updated patch (mozilla-central parts), this changes the styles as Neil suggested and updates to the latest code changes.
Attachment #329865 -
Attachment is obsolete: true
Assignee | ||
Comment 8•16 years ago
|
||
comm-central specific changes.
Assignee | ||
Comment 9•16 years ago
|
||
Carrying forward Neil's sr. Requesting r for the mail/ changes.
Attachment #330944 -
Attachment is obsolete: true
Attachment #331080 -
Flags: superreview+
Attachment #331080 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 10•16 years ago
|
||
Comment on attachment 330943 [details] [diff] [review]
[checked in] The fix (mozilla-central parts)
Carrying forward Neil's sr for the xpfe parts, requesting review for the toolkit changes.
Attachment #330943 -
Flags: superreview+
Attachment #330943 -
Flags: review?(enndeakin)
Comment 11•16 years ago
|
||
Comment on attachment 330943 [details] [diff] [review]
[checked in] The fix (mozilla-central parts)
OK, but please also test the highlightNonMatches getter as well. You can just add extra checks after the getAttribute check.
Attachment #330943 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 12•16 years ago
|
||
Comment addition Calendar - the theme item will only be required for the 1.8 branch. Request review so calendar also knows what I'm doing.
Attachment #331097 -
Flags: review?
Assignee | ||
Updated•16 years ago
|
Attachment #331097 -
Flags: review? → review?(daniel.boelzle)
Updated•16 years ago
|
Attachment #331097 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 13•16 years ago
|
||
Comment on attachment 331080 [details] [diff] [review]
[checked in] the fix (comm-central parts)
Magnus seems to be afk a bit at the moment.
Attachment #331080 -
Flags: review?(mkmelin+mozilla) → review?(bienvenu)
Updated•16 years ago
|
Attachment #331080 -
Flags: review?(bienvenu) → review+
Assignee | ||
Comment 14•16 years ago
|
||
mozilla-central: 16265:b6b0f1c44dc9
comm-central: 38:014093b71460
Also cross-committed to cvs for calendar.
Assignee | ||
Updated•16 years ago
|
Attachment #330943 -
Attachment description: The fix (mozilla-central parts) → [checked in] The fix (mozilla-central parts)
Assignee | ||
Updated•16 years ago
|
Attachment #331080 -
Attachment description: the fix (comm-central parts) → [checked in] the fix (comm-central parts)
Assignee | ||
Updated•16 years ago
|
Attachment #331097 -
Attachment description: Calendar comment change → [checked in] Calendar comment change
Assignee | ||
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a2
Comment 15•7 years ago
|
||
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/7c0511017c7a
Implement highlightNonMatches in toolkit autocomplete. r/sr=Neil for xpfe parts,r=enndeakin for toolkit parts
You need to log in
before you can comment on or make changes to this bug.
Description
•