Closed Bug 110081 Opened 23 years ago Closed 23 years ago

Add bookmark gets popup alert about URL invalid

Categories

(SeaMonkey :: Bookmarks & History, defect, P2)

Product:
SeaMonkey
Component:
Bookmarks & History
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.7

People

(Reporter: tracy, Assigned: paulkchen)

References

Details

(Keywords: smoketest)

Attachments

(2 files)

Patch hardens docshell against empty URLs
1.06 KB, patch
paulkchen
: review+
bugs
: superreview+
Details | Diff | Splinter Review
in OpenBookmarkURL(), if null url, don't load it, it's most likely a normal menu item
652 bytes, patch
adamlock
: review+
bugs
: superreview+
Details | Diff | Splinter Review 
seen on linux commercial build 2001-11-14-06-trunk

-launch the browswer and browse any page

-Click on Bookmarks | Add Bookmark

expected result:  the book mark is quietly added to the bookmark folder

tested result: the bookmark is added to the folder, but an Alert pops up saying 
"The URL is not valid and cannot be found" 

see also bug 110079 for Bookmark Manager adding bookmark on being called.
reminiscent of bug 108794 but not a dupe
this is also on windows commercial 2001-11-14-10-trunk
OS: Linux → All
Nightly 2001111403 on Windows ME also shows this. 

In addition, when the bookmark list is longer than the screen you get up and
down widgits to scroll the list up and down. As soon as you activate one of
those widgits, you get the same error, ie a popup alert saying "The URL is not
valid and cannot be loaded"


*** Bug 110191 has been marked as a duplicate of this bug. ***
*** Bug 110239 has been marked as a duplicate of this bug. ***
This happens on the Mac too so someone might want to change the OS to "All". 

Also, my bookmarks didn't show up for quite awhile. They showed up in the
Bookmarks Manager and my Personal Toobar so I knew it was reading my profile
info. They just didn't show up in the bookmarks dropdown menu.

I tried adding bookmarks and stuff (which didn't work as listed in this bug) and
then all of a sudden they just appeared. I don't know what caused it though.
P.S. - That was nightly Build ID: 2001111504 for Mac
Keywords: smoketest
Hardware: PC → All
*** Bug 110278 has been marked as a duplicate of this bug. ***
there was some confusion in one of the other dupes here about my global history
change - just to avert the blame, :) I only added an API, there are no callers
of hidePage() just yet.
*** Bug 110331 has been marked as a duplicate of this bug. ***
The same message appears when "File Bookmark... Ctrl-Shift-D" is selected in the
"Bookmarks" menu, but in this case the "Add bookmark" dialog appears as well.

Linux build 2001111506.
marking p2 and mozilla0.9.8
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.8
*** Bug 110381 has been marked as a duplicate of this bug. ***
This bug kind of, well, bugs me. It makes Mozilla unusable for day-to-day use.

Paul, do you mind if I have a try at fixing this (or at least tracking down the
problem) sometime in the next dew days? Do you have anything in mind for it already?


MAybe have a loot at bug 89118, which had a (I think) related checkin on nov 14
5:13 AM...
*** Bug 110419 has been marked as a duplicate of this bug. ***
*** Bug 110416 has been marked as a duplicate of this bug. ***
0.9.8 ???
Are you joking ? This is not only an "Add bookmark" problem
but also a scrolling problem as per comment #3.
The error message is a result of the checkin for bug 89118. The patch puts some
code into nsDocShell::LoadURI to display the error message when you tell it to
load a duff URL.

I'll set some breakpoints to see what URL is being fed into that method.
The reason for the error is that some JS is calling LoadURI with "" as the
specified URL. The fixup code is turning this into "http:///" (i.e. "http://" +
"" + "/"). This is an invalid URL so it doesn't get loaded.

This patch fixes docshell so it catches empty strings (and asserts), but I
still don't know what is calling it like this. It doesn't appear to be anything
in the handler for the Add Bookmark option.

Does anyone here know how I can turn a stack trace of JS calls into something
useful that points to the culprit?
I am no longer seeing this with commercial builds from this morning:

windows 2001-11-16-05-trunk
linux 2001-11-16-06-trunk
mac 2001-11-16-04-trunk


The bug is still there but something has changed so the that "http:///" is not
reported as a malformed URI so its not so obvious. It can still be seen from the
console.
WinME build 2001111603 the pop-up isn't appearing. However I do notice that the
scrolling of a long hotlist is now slower and the busy pointer is flickering on
and off while when your mouse is over the widget. Even if the bookmark list is
not scrolling at all (ie at top and bottom of list) it looks like something is
looping there.

Whatever it is it's leaking memory. I can put the mouse pointer over the "scroll
down" widgit and its gobbling about 100k per second. Even when it gets to the
bottom of the list and there is no scrolling, the mousepointer busy/flickering
is going, and memory usage is clocking up all the time.
Seeing the hourglass flickering too on both arrows.
BuildID 2001111603
When you are at one end of the list, trying to go further
makes the flicker very fast.
this is still "user" visible on Mas OSx build 2001-11-16-05-trunk
check that.....d'oh i'm retesting 11/15 build  I have no idea what todays build
is doing it's not complete yet.  sorry for the spammage
Can the target milestone been brought forward? It now looks like a serious
memory eating bug?
*** Bug 110553 has been marked as a duplicate of this bug. ***
All right, I think I've found the problem. There's an oncommand for the
bookmarks menus which does a OpenBookmarkURL(). This is what is calling
loadURL(). The fix is to look at the target and if it's not a bookmark, don't
friggin' try to load it! ;-)

I've also seen infinite calls to open the blank url on linux, at least. Marking
mozilla0.9.7
Target Milestone: mozilla0.9.8 → mozilla0.9.7
I bet the scrolling issue is that I had suggest the other day that the folder
pane could be scrolled faster if we give a longer timeout to popping up
subfolder content.
Comment on attachment 58203 [details] [diff] [review]
in OpenBookmarkURL(), if null url, don't load it, it's most likely a normal menu item

r=adamlock

Paul, can you review my patch? The extra checks to LoadURI should mean it
explicitly rejects empty strings rather than feeding them into various routines
until something returns an error.
Attachment #58203 - Flags: review+
Comment on attachment 58203 [details] [diff] [review]
in OpenBookmarkURL(), if null url, don't load it, it's most likely a normal menu item

sr=ben@netscape.com
Attachment #58203 - Flags: superreview+
Comment on attachment 58109 [details] [diff] [review]
Patch hardens docshell against empty URLs

sr=ben@netscape.com
Attachment #58109 - Flags: superreview+
Comment on attachment 58109 [details] [diff] [review]
Patch hardens docshell against empty URLs

r=pchen
Attachment #58109 - Flags: review+
fixed checked into trunk
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
My fix is checked in too. Other issues such as bookmark sidepanel flickering
deserve their own bug.
note to self:

scrolling, and bookmark flickering seems to be gone in 2001-11-18 w2k build.
2001111808 WinMe
The flickering hourglass cursor is gone, but the memory leak is still there.
Is this bug 93675 ?
*** Bug 110808 has been marked as a duplicate of this bug. ***
*** Bug 111469 has been marked as a duplicate of this bug. ***
*** Bug 110572 has been marked as a duplicate of this bug. ***
this has not been a problem for quite some time
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: