Open Bug 834691 Opened 8 years ago Updated 7 years ago

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

Categories

(SeaMonkey :: General, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

People

(Reporter: ewong, Unassigned)

Details

Attachments

(1 file)

No description provided.
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)
Attached patch patch v1Splinter Review
wip patch that needs a bit of feedback.
Attachment #719326 - Flags: feedback?(philip.chee)
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)
> 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 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 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-
> +        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()
Assignee: ewong → nobody
Status: ASSIGNED → NEW
Assignee: nobody → ewong
Status: NEW → ASSIGNED
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.