201235 - Quicksearch is broken: load_absolute_url is not defined

Status: VERIFIED INVALID

(Bugzilla :: Bugzilla-General, defect)

Description

[Gervase Markham [:gerv]]

QuickSearch (the one embedded in web pages, not the sidebar one) is broken on
the trunk. It appears to have been broken by the checkin for bug 153874, which
added this function to sidebar.xul.tmpl. 

The function is therefore not available when QuickSearch is used from a web
page, e.g. on the guided enter_bug format:
http://bugzilla.mozilla.org/enter_bug.cgi?product=Browser&format=guided 
(but note that it works on b.m.o. because we haven't upgraded it since it got
broken.)

[e-mail]: could you sort this out, please? :-)

CCing justdave for an assessment of this bug's relevance to the next release.

Gerv

Comment 1

[Peter Henderson]

Oops, I forgot quicksearch.js was used by other files besides the sidebar. :o)

All you have to do is change load_relative_url() to load_absolute_url() in the
template for enter_bug.cgi. I'll have a patch as soon as I get my local copy cvs
up'd to the trunk. I'll make sure there aren't any other remaining references to
load_relative_url that mean absolute_url while I'm at it as well.

Comment 2

[Peter Henderson]

Attachment: Change the other occurence of load_relative_url() to load_absolute_url()

Try this. I just changed "relative" to "absolute" (http://b.m.o/xxx isn't
relative now is it? :o). I tried to make the comment more clear but it could
use more work so it makes even more sense and isn't redundant. I have to run
but I can fix it when I get back if no one else wants to. :o)

Comment 3

[Peter Henderson]

Attachment: Patch with better comment

I think this comment is more clear. The code change is still just the one line
(relative -> absolute).

Comment 4

[Peter Henderson]

Comment on attachment 119932 [details] [diff] [review]
Patch with better comment

kiko, you reviewed my original patch from bug 153874. Does this look ok?

Comment 5

[Christian Reis]

r=kiko for the quick fix

Why there is no comment in quicksearch.js:go_to() explaining that
load_absolute_url() *must* be defined in the calling page? I think I missed this
in the original review. Anyway, it would be nice to add it there, and I'm sure
you don't need to open a new bug just for that.

Comment 6

[Christian Reis]

Comment on attachment 119932 [details] [diff] [review]
Patch with better comment

ping?

Comment 7

[Vlad Dascalu]

Since pinging the author doesn't seem to work, I suggest to checkin this and 
open another bug for what kiko wants.

Comment 8

[Dave Miller [:justdave]]

ok, that works for me.

Comment 9

[Christian Reis]

I really don't like the way sidebar = 1 is being abused there, though I realize
unbreaking quicksearch is rather urgent-looking. Added an XXX comment to the
code and filed bug 236025 for the cleanup.

Comment 10

[Vlad Dascalu]

Comment on attachment 119932 [details] [diff] [review]
Patch with better comment

Our codebase changed a lot in the last year, therefore the current patch can no
longer be applied.

Comment 11

[Vlad Dascalu]

Invalid patch, nothing to be checked in.

Comment 12

[Christian Reis]

Err, what do you mean, "invalid"?

Comment 13

[Vlad Dascalu]

I get, using grep:

[vladd@abacron bugzilla]$ grep -R "function load_absolute" .
./template/en/default/sidebar.xul.tmpl:function load_absolute_url( aAbsoluteURL ) {


And inside the sidebar.xul.tmpl, the only place where this is found, the code
looks like this:

// Tell QuickSearch that the source of this is the sidebar
var sidebar = 1;

function load_absolute_url( aAbsoluteURL ) {
    content.location = aAbsoluteURL;
}

function load_relative_url( aRelativeURL ) {
    aRelativeURL = '[% Param('urlbase') %]' + aRelativeURL;
    _content.location = aRelativeURL;
}

which looks pretty different from what we have in the proposed patch.

Comment 14

[Vlad Dascalu]

Ok, s/invalid/obsolete/.

Comment 15

[Christian Reis]

Myk removed quicksearch from that page in lieu of full-text searching.

Comment 16

[Christian Reis]

For reference:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=create-guided.html.tmpl&branch=&root=/cvsroot&subdir=mozilla/webtools/bugzilla/template/en/default/bug/create&command=DIFF_FRAMESET&rev1=1.10&rev2=1.11

Comment 17

[Christian Reis]

Sorry, flubbed the knob. But it is invalid :-)

Comment 18

[Vlad Dascalu]

clearing target in DUPLICATE/WORKSFORME/INVALID/WONTFIX bugs so they'll show up
as untriaged if they get reopened.