Find doesn't ignore elements styled as display:none (finds hidden/invisible elements)

NEW
Assigned to

Status

()

defect
P5
major
14 years ago
3 years ago

People

(Reporter: u49640, Assigned: martijn.martijn)

Tracking

({testcase})

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

()

Attachments

(3 attachments)

Reporter

Description

14 years ago
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
Reporter

Comment 1

14 years ago
(changing it to Find toolbar, i think it should be over there)
Component: Help Documentation → Find Toolbar / FastFind
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. 
Assignee

Comment 3

14 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

14 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
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.
Assignee

Comment 6

14 years ago
Posted file testcase
Assignee

Comment 7

14 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?
Reporter

Comment 8

14 years ago
(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)
Assignee

Comment 9

14 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

14 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.
Assignee

Comment 11

14 years ago
Posted patch patchSplinter Review
This does what we want here, but it isn't any good, because it also ignore
visibility:hidden and that should not happen.
Assignee

Comment 12

14 years ago
Posted file testcase2
With the patch, it doesn't skip the first instances of "word" in this testcase.

Comment 13

14 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".
Duplicate of this bug: 407234
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)
Assignee

Comment 15

12 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
Product: Firefox → Toolkit

Comment 16

11 years ago
Also happens on GNU/Linux.
Reporter

Comment 17

11 years ago
-> All/All
OS: Windows XP → All
Hardware: PC → All

Updated

10 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

Updated

9 years ago
Duplicate of this bug: 565651
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

4 years ago
See Also: → 822458
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.