Closed
Bug 305381
Opened 19 years ago
Closed 2 years ago
Find doesn't ignore elements styled as display:none (finds hidden/invisible elements)
Categories
(Core :: Find Backend, defect, P5)
Core
Find Backend
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: u49640, Unassigned)
References
()
Details
(Keywords: testcase)
Attachments
(3 files)
309 bytes,
text/html
|
Details | |
1.32 KB,
patch
|
Details | Diff | Splinter Review | |
487 bytes,
text/html
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050820 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b4) Gecko/20050820 Firefox/1.0+
Sometimes the Findfast search doesnt work in the Help viewer (unfortunatly it
didnt happen all the time, but sometimes)
Reproducible: Sometimes
Steps to Reproduce:
1.Open The keyboard shortcut help page
2. open findfast
3. enter "alt"
--> the first text with "alt" is selectet"
enter "+"
nothing is found, but there are a lot of "Alt+" strings in there
or
search for "ctrl+" works, but "ctrl+l" doesnt (sometimes this one did find
something)
Expected Results:
a search for "alt+" should work and "ctrl+l" should work too
(changing it to Find toolbar, i think it should be over there)
Component: Help Documentation → Find Toolbar / FastFind
Comment 2•19 years ago
|
||
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20050821
Firefox/1.6a1 ID:2005082101
Yes, I see the same.
Comment 3•19 years ago
|
||
It's this code:
<span class="win">
<kbd class="noMac">
Alt </kbd>
<kbd class="mac">
Opt </kbd>
+ <kbd>
Right Arrow </kbd>
<br/>
<kbd>
Shift </kbd>
+ <kbd>
Backspace </kbd>
</span>
So on windows, the class="mac" is invisible.
But that's not something that the find function can 'see'.
Comment 4•19 years ago
|
||
Indeed. It finds "AltOpt+".
http://lxr.mozilla.org/seamonkey/source/browser/locales/en-US/chrome/help/platformStrings.dtd#31
http://lxr.mozilla.org/seamonkey/source/toolkit/components/help/content/platformClasses.css#38
Status: UNCONFIRMED → NEW
Ever confirmed: true
QA Contact: help.documentation → fast.find
Summary: Findfast doesnt allways work in the Help viewer → Find doesn't ignore <span>s styled as display:none
Comment 5•19 years ago
|
||
This also would affect searches for "option" or "preference", as they're defined
similarly to how shortcut keys are defined.
Note the help viewer could probably work around this by removing elements which
aren't displayed in the current help doc using a script like the one below (may
not work, I've never used tree walkers before):
<script type="application/x-javascript">
var searchRegexp = new RegExp( "\b(" +
#ifdef XP_WIN
"noWin|mac|unix" +
#else
#ifdef XP_MACOSX
"noMac|win|unix" +
#else
#ifdef XP_OS2
"noWin|mac|unix" +
#else
"noUnix|win|mac" +
#endif
#endif
#endif
")\b" );
var filter = {
acceptNode: function(node) {
if (node.getAttribute("class").search(searchRegexp) < 0)
return NodeFilter.FILTER_REJECT;
else
return NodeFilter.FILTER_ACCEPT;
}
};
var elemsToHide = document.createNodeIterator(document.documentElement,
NodeFilter.SHOW_ELEMENT,
filter,
true);
var nextNode;
while(elemsToHide.nextNode()) {
nextNode = elemsToHide.currentNode;
nextNode.parentNode.removeChild(nextNode);
}
</script>
However, this errant behavior is an edge case (based on the number of times
someone's reported the bug), and I'm not sure I would even make a case for the
workaround in trunk code, let alone branch code.
Comment 6•19 years ago
|
||
Comment 7•19 years ago
|
||
Ok, IE6 acts the same as Mozilla, so I don't think that is a bug.
So the help viewer needs to get fixed.
(In reply to comment #5)
createNodeIterator is not implemented in Mozilla.
Something like this is working:
<script>
window.onload = function() {
var nodes = document.getElementsByTagName('body')[0].getElementsByTagName('*');
var nodes_length = nodes.length;
for (var i=nodes_length-1; i >= 0; i--) {
if (window.getComputedStyle(nodes[i], '').getPropertyValue('display') == 'none')
nodes[i].parentNode.removeChild(nodes[i]);
}
}
</script>
But it makes things slower.
I think it is best to not insert those nodes when not necessary.
Isn't it possible to use ifdefs in platformStrings.dtd instead of in
platformStrings.css?
(In reply to comment #7)
> Ok, IE6 acts the same as Mozilla,
We can do better than IE
> so I don't think that is a bug.
No, it should find the stuff on the screen. (i dont think that anyone wants to
find hidden Text. its hidden for a reason)
or is there any reason why hidden Text is included in the search?
(since there is no (normal) way of showing the search result its pretty much not
usefull to search for it)
Comment 9•19 years ago
|
||
Mozilla doesn't find stuff that is invisible (or display:none), so that's not
the issue here (the summary is probably not a good one here).
The issue is that a word that is separated by a display:none element, isn't found.
Now, you could call that a bug (although it might not be).
But we probably need a separate bug, because the help viewer issue can be fixed
without this issue.
Comment 10•19 years ago
|
||
> Isn't it possible to use ifdefs in platformStrings.dtd instead of in
> platformStrings.css?
#ifdefs are verboten in locale files, because langpacks ought to be
cross-platform. And we use the platform classes in help content as well, just
look at
http://lxr.mozilla.org/seamonkey/source/browser/locales/en-US/chrome/help/shortcuts.xhtml#53
IMHO fixing the help viewer would only workaround this find toolbar bug.
Comment 11•19 years ago
|
||
This does what we want here, but it isn't any good, because it also ignore
visibility:hidden and that should not happen.
Comment 12•19 years ago
|
||
With the patch, it doesn't skip the first instances of "word" in this testcase.
Comment 13•19 years ago
|
||
> This does what we want here, but it isn't any good, because it also ignore
> visibility:hidden and that should not happen.
Why shouldn't it? IsVisibleNode() was introduced with rev. 1.15 of nsFind.cpp,
checkin comment was "Bug 175046, bug 172991, bug 166471. Make sure typeaheadfind
and regular find don't find comment nodes, display:none, visibility:hidden or
visibility:collapsed nodes".
Updated•17 years ago
|
Summary: Find doesn't ignore <span>s styled as display:none → Find doesn't ignore <span>s styled as display:none (finds hidden/invisible elements)
Comment 15•17 years ago
|
||
(In reply to comment #13)
> > This does what we want here, but it isn't any good, because it also ignore
> > visibility:hidden and that should not happen.
> Why shouldn't it?
You're right, I don't really know what I was thinking here.
Assignee: nobody → martijn.martijn
Assignee | ||
Updated•16 years ago
|
Product: Firefox → Toolkit
Comment 16•16 years ago
|
||
Also happens on GNU/Linux.
Updated•15 years ago
|
Keywords: testcase
Summary: Find doesn't ignore <span>s styled as display:none (finds hidden/invisible elements) → Find doesn't ignore elements styled as display:none (finds hidden/invisible elements)
Version: unspecified → Trunk
Comment 19•12 years ago
|
||
I just hit this on http://taras.glek.net/blog/2013/01/04/snappy-2012-summary/ by searching for "bug 699820"
Is this considered a low-priority issue?
Updated•8 years ago
|
Priority: -- → P5
Comment 22•2 years ago
|
||
The bug assignee didn't login in Bugzilla in the last months and this bug has severity 'major'.
:enndeakin, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee: martijn.martijn → nobody
Flags: needinfo?(enndeakin)
Updated•2 years ago
|
Component: Find Toolbar → Find Backend
Flags: needinfo?(enndeakin)
Product: Toolkit → Core
Comment 23•2 years ago
|
||
Pretty sure this works now: https://searchfox.org/mozilla-central/rev/7b9d23ece4835bf355e5195f30fef942d376a1c7/toolkit/components/find/nsFind.cpp#182-187
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
You need to log in
before you can comment on or make changes to this bug.
Description
•