Closed Bug 493658 Opened 15 years ago Closed 15 years ago

Highlight all of the Findbar does not work in a certain page

Categories

(Toolkit :: Find Toolbar, defect)

Product:
Toolkit
Component:
Find Toolbar
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: alice0775, Assigned: martijn.martijn)

References

(
URL
)

Details

(Keywords: regression, testcase, verified1.9.1)

Attachments

(2 files)

testcase
197 bytes, text/html
Details
patch
7.68 KB, patch
vlad
: review+
vlad
: superreview+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090518 Firefox/3.5.0 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090518 Minefield/3.6a1pre ID:20090518133806

Highlight all of the Findbar does not work in a certain page

Reproducible: Always

Steps to Reproduce:
1.Start Mine field with new porofile
2.Go URl
3.CTRL+F
4.Input "sidebar" in the FindbAr
5.Turn on Highlight all button in the Findbar

Actual Results:  
Highlight all does not work.

Expected Results:  
Highlight all  should work.

Error in the error console:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIInterfaceRequestor.getInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: chrome://global/content/bindings/findbar.xml :: _getSelectionController :: line 458"  data: no]

And Shiretoko is broken too.
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090518 Shiretoko/3.5b5pre ID:20090518142934
Confirming as also an issue under Linux.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Regression range:

Works fine:
http://hg.mozilla.org/mozilla-central/rev/bbc07a9bf556
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080925033548 Minefield/3.1b1pre

Broken:
http://hg.mozilla.org/mozilla-central/rev/2e7f03bc1820
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080926033937 Minefield/3.1b1pre

Pushlog between the above regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bbc07a9bf556&tochange=2e7f03bc1820

Candidate for regression:
Bug 429723 - Empty findbar turns red and "Phrase not found" is displayed when "Highlight all" is enabled and search term is removed
Blocks: 429723
Keywords: regression
another candidate for regression:
Bug 451286 - 'Highlight all' does not seem to work at all across frames; r=mano
The following modification for findbar.xml fix this problem.

      <method name="_getSelectionController">
        <parameter name="aWindow"/>
        <body><![CDATA[
          // Yuck. See bug 138068.
          var Ci = Components.interfaces;
          var docShell = aWindow.QueryInterface(Ci.nsIInterfaceRequestor)
                                .getInterface(Ci.nsIWebNavigation)
                                .QueryInterface(Ci.nsIDocShell);

+          try {
            var controller = docShell.QueryInterface(Ci.nsIInterfaceRequestor)
                                   .getInterface(Ci.nsISelectionDisplay)
                                   .QueryInterface(Ci.nsISelectionController);
+          } catch (e) {
+            return null;
+          }
          return controller;
        ]]></body>
      </method>
Blocks: 451286
No longer blocks: 429723
Component: General → Find Toolbar
Product: Firefox → Toolkit
QA Contact: general → fast.find
Attached file testcase — 
It happens when a display: none iframe is in the page.
You also get this error message in the error console then:
Error: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsIInterfaceRequestor.getInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "JS frame :: chrome://global/content/bindings/findbar.xml :: _getSelectionController :: line 458"  data: no]
Attached patch patch — — Splinter Review 
It seems to me this would be the correct solution. 
When there is no innerWidth or innerHeight on the window, it doesn't make much sense to highlight anything in the window, anyway.
Attachment #378322 - Flags: review?(graememcc_firefox)
Flags: blocking1.9.1?
Keywords: testcase
Flags: blocking1.9.1? → blocking1.9.1+
Assigning to Martijn. I think we might need to find another reviewer, though. Maybe Connor?
Assignee: nobody → martijn.martijn
Whiteboard: [needs review]
Comment on attachment 378322 [details] [diff] [review]
patch

I guess Mano might be able to review this. But I think Graeme should also look at this, because he knows this code the best.
Attachment #378322 - Flags: review?(mano)
Attachment #378322 - Flags: review?(mano)
Attachment #378322 - Flags: review?(graememcc_firefox)
Attachment #378322 - Flags: review+
Comment on attachment 378322 [details] [diff] [review]
patch

This seems fine to me, though I wonder if we should also put the try/catch block in there in case we run into another similar problem; if we can't get a selection controller, we can't do any selection anyway, right?
Whiteboard: [needs review] → [can land]
on trunk: http://hg.mozilla.org/mozilla-central/rev/021ff461c9df

patch does not apply on branch.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [can land] → [needs branch patch?]
thought it was a format issue, converted to unix, but still rejected, so manually recreated the patch.

on branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5ac037ba09f9
Keywords: fixed1.9.1
Whiteboard: [needs branch patch?]
Verified in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1pre) Gecko/20090526 Shiretoko/3.5pre & Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090526 Minefield/3.6a1pre
Keywords: fixed1.9.1 → verified1.9.1
Status: RESOLVED → VERIFIED
The patch modified an existing automated test to also test this case automatically.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: