The default bug view has changed. See this FAQ.

Ctrl+F shouldn't work to open Find toolbar in about:permissions for realz this time.

RESOLVED FIXED in mozilla18

Status

()

Toolkit
Find Toolbar
RESOLVED FIXED
6 years ago
4 years ago

People

(Reporter: Mardeg, Assigned: graememcc)

Tracking

Trunk
mozilla18
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
This is "to address the remaining issues" bug 661823 didn't.
STR:
1. Launch Firefox
2. Go to about:permissions
3. Press CTRL+F

Result: Find toolbar opens.
(Reporter)

Updated

6 years ago
OS: Windows XP → All
Hardware: x86 → All
(Assignee)

Comment 1

5 years ago
This isn't working - too early?
http://hg.mozilla.org/mozilla-central/file/0c7e2911be75/browser/base/content/browser.js#l4842

Dump debugging is telling me at this point, readystate is uninitialised, and though content.document is the XULDocument, it doesn't have any children or siblings at that point, and content.document.documentElement is null. Which of course, makes one of the if conditions fail, and leaves disableFind at false.

And it's broken in new tab, and about:config too.

Can the check go in endDocumentLoad, or is that too late?
(Assignee)

Comment 2

5 years ago
In fact, I think the only reason this even works for about:addons is because of the way addons loads content for the various panels after extensions.xul has loaded - we call onLocationChange a second time, and at this point we've loaded enough of extensions.xul to find the disableFastFind attribute.
(Assignee)

Comment 3

5 years ago
Created attachment 668021 [details] [diff] [review]
v1

> Can the check go in endDocumentLoad, or is that too late?
No it can't. These aren't network loads.

This seems to do the trick. I had to modify the test slightly for about:addons to wait for the correct load event.

Note that this doesn't solve the new tab page - at least not when it's launched from the new tab button (the patch works fine when manually typing about:newtab). Newtab currently fails (and continues to fail with this patch) for a different reason from the other pages - tabbrowser's mTabProgressListener suppresses onLocationChange calls for the new tab page, so we never get an opportunity to disable find. I can't see an easy fix for this.

With preload true, this seems to only be a problem the first time the new tab page is displayed - it works once the page is preloaded and swapped in.
Attachment #668021 - Flags: review?(gavin.sharp)
Comment on attachment 668021 [details] [diff] [review]
v1

Thanks for cleaning this up - it's much better now!

>diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js

>   onLocationChange: function (aWebProgress, aRequest, aLocationURI, aFlags) {

>       // Disable find commands in documents that ask for them to be disabled.
>       if (aLocationURI &&
>           (aLocationURI.schemeIs("about") || aLocationURI.schemeIs("chrome"))) {
>+        // Ignore location changes for replaceStates in about:addons

nit: I don't like the phrasing of this comment, because it implies those are the only cases that matters (they may be the only known cases, but this code would like to ignore all SAME_DOCUMENT location changes). I would prefer:

// Don't need to re-show/hide find comments for same-document location changes
// (e.g. the replaceStates in about:addons)

>+  shouldDisableFind: function(aDocument) {

>+  disableFindCommands: function (aDisable) {

Can you just define these in onLocationChange, rather than putting them on XULBrowserWindow? That object is already kind of polluted with all sorts of unrelated stuff, I don't want to make that worse.
Attachment #668021 - Flags: review?(gavin.sharp) → review+
(Assignee)

Comment 5

5 years ago
> Can you just define these in onLocationChange, rather than putting them on XULBrowserWindow?
I took the opportunity to also move the event listener, to tidy up that code block.

https://hg.mozilla.org/integration/mozilla-inbound/rev/46581931a047
Assignee: nobody → graememcc_firefox
Status: NEW → ASSIGNED
(Assignee)

Comment 6

5 years ago
And because I can be rely on to screw things up:
https://hg.mozilla.org/integration/mozilla-inbound/rev/34cc2b570666
https://hg.mozilla.org/mozilla-central/rev/46581931a047
https://hg.mozilla.org/mozilla-central/rev/34cc2b570666
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
(Assignee)

Comment 8

5 years ago
See comment 6:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4db6171d4fac
https://hg.mozilla.org/mozilla-central/rev/4db6171d4fac
Depends on: 843432
You need to log in before you can comment on or make changes to this bug.