Port |Bug 632233| to SeaMonkey (XULBrowserWindow.onLocationChange looks for a "disablefastfind" attribute in all content documents before ruling them out)

NEW
Unassigned

Status

SeaMonkey
General
5 years ago
5 years ago

People

(Reporter: ewong, Unassigned)

Tracking

Trunk
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Comment hidden (empty)

Comment 1

5 years ago
Bug 569342 - Find bar should not be enabled in about:addons.
Bug 632233 - XULBrowserWindow.onLocationChange looks for a "disablefastfind" attribute in all content documents before ruling them out because their URI scheme isn't "about" or "chrome".
Summary: Port |Bug 632233| to SeaMonkey → Port |Bug 632233| to SeaMonkey (XULBrowserWindow.onLocationChange looks for a "disablefastfind" attribute in all content documents before ruling them out)
(Reporter)

Comment 2

5 years ago
Created attachment 719326 [details] [diff] [review]
patch v1

wip patch that needs a bit of feedback.
Attachment #719326 - Flags: feedback?(philip.chee)

Comment 3

5 years ago
Comment on attachment 719326 [details] [diff] [review]
patch v1

> +      // Utility functions for disabling find
> +      var shouldDisableFind = function shouldDisableFind(aDocument) {
Just pass the documentElement directly here.

> +        let docElt = aDocument.documentElement;
use var instead of let here and in the rest of this patch.

> +      var disableFindCommands = function disableFindCommands(aDisable) {
> +        let findCommands = [document.getElementById("cmd_find"),
> +                            document.getElementById("cmd_findAgain"),
> +                            document.getElementById("cmd_findPrevious")];

I see that we use the following:
"cmd_findTypeText", "cmd_findTypeLinks", "cmd_find", 'cmd_findNext", "cmd_findPrev"

In addtion I think "cmd_findTypeText" and "cmd_findTypeLinks" are controlled by nsTypeAheadFind.js 

> +        findCommands.forEach(function(elt){
> +          if (elt) {
I think we can be sure that these elements always exist.

I'm not sure how we differ from Firefox but if you open two tabs:
http://www.mozilla.org/en-US/ and
about:config

The Edit-> Find* commands are initially disabled for about:config, but if you tab to another tab and then back to about:config the Edit->Find* menu items are enabled.

I'm lost so am passing the buck to Neil.
Attachment #719326 - Flags: feedback?(philip.chee)

Comment 4

5 years ago
> I'm lost so am passing the buck to Neil.
Hmm if I remove this condition:
>        if (!(aFlags & Components.interfaces.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) {
Then the Find/FindAgain/FindPrev menu items are updated properly.

Comment 5

5 years ago
Comment on attachment 719326 [details] [diff] [review]
patch v1

CC Neil for feedback.

> In addtion I think "cmd_findTypeText" and "cmd_findTypeLinks" are controlled
> by nsTypeAheadFind.js

> Hmm if I remove this condition:
>>        if (!(aFlags & Components.interfaces.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) {
> Then the Find/FindAgain/FindPrev menu items are updated properly.
Attachment #719326 - Flags: feedback?(neil)

Comment 6

5 years ago
Comment on attachment 719326 [details] [diff] [review]
patch v1

I don't like this readystatechange business. As it happens, we already have a way of detecting when a document load ends, there's a function called endDocumentLoad that we can use. As for avoiding unnecessary same document checks, I think you'll find that aRequest is null for a tab switch.
Attachment #719326 - Flags: feedback?(neil) → feedback-

Comment 7

5 years ago
> +        if (!(aFlags & Components.interfaces.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) {
> +          if (content.document.readyState == "interactive" || content.document.readyState == "complete") {

So if I understand Neil correctly this should be:

        if (!aRequest ||
            !(aFlags & nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)) {
          if (content.document.readyState == "interactive" || content.document.readyState == "complete") {

Still don't know about endDocumentLoad()
(Reporter)

Updated

5 years ago
Assignee: ewong → nobody
Status: ASSIGNED → NEW
(Reporter)

Updated

5 years ago
Assignee: nobody → ewong
Status: NEW → ASSIGNED
(Reporter)

Comment 8

5 years ago
Gone back to square one, so this bug will have to have someone else to give it love.  I might retake this bug when I feel I have the brain cells to match the required skill.  Until then, this is available for anyone to take.
Assignee: ewong → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.