77635 - When offline/tab not found, sidebar should indicate that in tab

Status: VERIFIED FIXED

(SeaMonkey :: Sidebar, defect, P2)

Description

[Viswanath Ramachandran]

When you go offline, or if you fail to get the tab, instead of the XUL alert 
dialog saying Page not Found, that message needs to go into the sidebar tab 
itself.

Comment 1

[Viswanath Ramachandran]

Matt - have we already worked on this or is there a dupe that I am not aware of? 
thanks, Vishy

Anatoliy - can you speak to Matt on what we've already accomplished on this and 
directions to look at. This may be a hard bug as I'm not sure that the current 
FE controls the "page not Found" dialog. 

Comment 2

[Blake Ross]

Anatoliy, you *might* be able to do this by implementing nsIWebProgressListener 
and handling errors yourself in endDocumentLoad.  But I don't know if you can 
intercept the alert; as Vishy said, this will be hard because docshell throws 
it before we take control.  See 
http://lxr.mozilla.org/seamonkey/source/docshell/base/nsWebShell.cpp#1104

Comment 3

[Blake Ross]

With regards to nsIWebProgressListener, see how we handle it in the browser in 
nsBrowserStatusHandler.js for clues on how to do this.

Comment 4

[Anatoliy Averbukh]

working on this bug.

Comment 5

[selmer (gone)]

I would not rate this critical for the beta.  If the warning dialog is correct,
that would be sufficient if inelegant.  Is this really the most important bug to
be working on for the beta??

Comment 6

[benc]

Are you planning on fixing this so bug 48664 and 63859 work?

The fix for this problem needs to be tested against file, http and chrome URLs.

Comment 7

[Viswanath Ramachandran]

Kevin: I'd like to cut this for the beta, we're trying to get the list down to 
an absolute musthave list. moving to mozilla0.9.2. 

Comment 8

[matt]

Anatoliy,

i talked to gagan about this bug and he said he wanted to help if it's a 
networking problem.  Make sure you sync up with him on this.

Comment 9

[Anatoliy Averbukh]

The main problem with this bug is concerned with the following:
there is no dialog for the cases described by Vishy.
And user can be confused, when it happenes.
So I have added that dialog for those cases.
See attachment below.

Unfortunately, it is not so easy to fix problem with nsWebShell, because the 
problem is concerned with some Connect() error and  nsWebShell doesn't handle 
that types of errors (in fact, it considers the situation as a normal one).
And also, I am not sure, if it should be handled there.

So I propose local (just for sidebar) solution.

Comment 10

[Anatoliy Averbukh]

Attachment: patch for the bug

Comment 11

[matt]

r=matt if gagan has talked to you.  I want to make sure he doesn't need to fix 
anything on the networking side.

Comment 12

[matt]

I talked to gagan and he said you haven't talked to him.
Please do that since he knows the networking stuff and
we might just creating a hack that is glossing over a real
netlib bug.  Thanks.

Comment 13

[Anatoliy Averbukh]

Doing that. Let you know about the results.

Comment 14

[Viswanath Ramachandran]

no, we dont want another alert dialog. What I want is that the "Page not found" 
message instead of coming up as an alert, needs to go into the sidebar tab 
display itself. In other words, the sidebar FE needs to capture this event from 
whoever is putting up the dialog (I think necko) and register itself to handle 
this event differently. 

Comment 15

[Anatoliy Averbukh]

Attachment: I deleted alert dialog and now it goes to sidebar

Comment 16

[matt]

After talking to gagan I still think this is a hack.  It might work but it's nto 
the "right" fix.   We should ship this in beta and see how it goes.  Then we 
should talk to gagan and juds team about letting the front end suppress dialogs.  
I image we can do it though xpcom and that is what we should be doing.

Thanks anatoly for doing this.  It's a really smart hack to hide the problem.
Let's either close this bug and file a new one to fix it the right way or leave 
this one open.  and just lower the priority.

cc'ing valeksi to ask if he could point us as to how to supress a dialog and 
estimate how hard it is if we aren't doing it already.

Comment 17

[Anatoliy Averbukh]

Matt, 
I spoke to Gagan yesterday and we determined the following 2 problems:
-  send message to sidebar (it is done with the patch);

-  to generate some event from necko
  (And I am still trying to do that now, didn't finish yet).

Comment 18

[Viswanath Ramachandran]

Anatoliy, 
pls continue work on this - but we can only check in after beta i.e. when the
mozilla tip is open again for m0.9.2 checkins. I have some concerns about the
latest patch:
1. sidebar shd not add code to editor directories
2. the message shd not be in html, it has to use XUL and the message string
itself shd be in a localizable dtd or properties file

thanks, Vishy

Comment 19

[benc]

should be a depends on necko actually giving errors to URL's that don't exist

Comment 20

[Anatoliy Averbukh]

Attachment: patch for the bug

Comment 21

[Anatoliy Averbukh]

In according to Vishy comments, I changed the following:
1. I am loading a xul file (not HTML) and text string is localizable now,
2. I added an xul file to sidebar directory (instead of editor)
 

Comment 22

[Paul Chen]

Anatoliy, can you attach PageNotFound.xul? It wasn't included in the last diff
because it's a new file. Thanks.

Comment 23

[Anatoliy Averbukh]

Attachment: added file PageNotFound.xul

Comment 24

[Matthew T (active 1999-2002)]

I don't see the point of fixing this just for the sidebar, when fixing bug 28586
would solve the problem everywhere and would probably be the same amount of
work. I suggest this bug be marked as a duplicate of that one.

Comment 25

[James Green]

Applies to All/All.

Comment 26

[Paul Chen]

I'd suggest changing sidebar.pagenotfound.label to "Page Not Found" (I believe
web servers call 404 'Not Found'). Also, I'd
really like gagan/valeski to fix the real underlying problem, but until that
gets fixed, this leaves us hacking around it in the front end, sigh.

r=pchen

Comment 27

[Viswanath Ramachandran]

to be more specific, we must change the string in the dtd to "Page Not Found", 
before checking this in. 

Comment 28

[Anatoliy Averbukh]

Attachment: changed string in dtd

Comment 29

[Anatoliy Averbukh]

Please SR.

Comment 30

[Viswanath Ramachandran]

Who are you asking to SR? 

Comment 31

[Alec Flett]

why can't we say something like "This panel could not be loaded. Please try
again later" or something? "Page Not Found" is very unfriendly and doesn't
actually explain the problem.

as for the patch, you cannot use <html:h2> - instead, you should be using
<html>, and using CSS to style the text.

Comment 32

[Alec Flett]

also:
- you need to declare timeoutID somewhere - it appears to be a global variable,
so please prefix it with "g" such as "gTimeoutID"
- you're passing in "setBlank();" when you can actually pass in a pointer to the
function - as in setTimeout(setBlank, 20000) - that will save an eval

Comment 33

[kmurray1115]

Not a bad idea Alec - Although the term "Panel" is not what we should use. The
context menu using "Panel" (Hide/Switch) is inconsistent with the tab naming
we've gone with, and I'd like to get that fixed. But in this case something more
descriptive would definitely be better. Here are some options:
* "Tab Not Found"
* "Content Not Found"
* "Tab content could not load - please try again"

The third one seems best, but I'm just a little worried about trying to shove
too much descriptive language into the tab content area, making it look like a
dialog :-(

Other thoughts?

Comment 34

[Alec Flett]

as discussed with anatoliy on AIM, he should be using <html class="header"> to
get the appearance he desires. This is defined in each skin as having a bold
appearance.

Comment 35

[Anatoliy Averbukh]

Attachment: new patch

Comment 36

[Anatoliy Averbukh]

In the new patch, I made minor changes in the code (naming for global variables, 
passed reference instead of string ).

I changed and PageNotFound.xul as we agreed with Alec.
However, I still needed to set style to provide bold view.
Definitely, there is no reason to provide skinnability for this, in fact 
internal, xul file)

Comment 37

[Alec Flett]

sorry, you cannot use inline style - i.e. the style="font-weight: bold"
<html class="header"> should be enough to make it bold, no?

also, I personally don't think "Tab Content Not Found" is any more informative 
than "Page Not Found." My complaint is with the "Not Found" part: there are 
multiple reasons that a tab won't load, and only a small percentage of the time 
is that it's "not found" More often it's that you are offline and the page 
couldn't be loaded, sometimes the connection to the server times out, who knows. 
In any case, we also need to indicate to the user that this is likely a 
temporary condition, and that they should expect to see the tab loaded at 
another point.

How about "This Tab is not available right now."?

Regarding this, we should file another bug about putting in a "refresh" button 
which tries to load the tab again, so that the tab doesn't stay in this 
permenent state of not being loaded.

Comment 38

[Alec Flett]

also, EVERY xul file is an "internal file" - therefor there are not exceptions 
to the no-inline style rules... the existing inline style which you may have 
found in the tree is not something to mimic - it is something that needs to be 
corrected at a future point

Comment 39

[kmurray1115]

<<How about "This Tab is not available right now."?>>

Sounds good to me. The refresh bug is part of another bug (bug 77635) "adding a
stop button to Sidebar" which would require a reload after a user stops.

Comment 40

[Anatoliy Averbukh]

Attachment: Updated title and CSS for the title ("header" class doesn't provide BOLD)

Comment 41

[Alec Flett]

ooooh! I see what's going on now.
you need to pull in chrome://communicator/skin/ as a stylesheet to 
pagenotfound.xul, then class="header" will work.

Comment 42

[Viswanath Ramachandran]

Anatoliy - "The Tab Not Available Now" is not an acceptable string, its not well
formed. Please stick with what Alec/KMurray suggested i.e. "This tab is not
available right now". Alec/Kevin - I think it shd be "This tab ..." not "This
Tab ..." - is that okay? thanks! Vishy

Comment 43

[Anatoliy Averbukh]

Attachment: Thank you Alec, it is working now! (changed and title string)

Comment 44

[Alec Flett]

last issue - shouldn't there be a "." at the end of the sentence?
sr=alecf with that change.

Comment 45

[Asa Dotzler [:asa]]

a= [e-mail] for checkin to the trunk.
(on behalf of drivers)

Comment 46

[Anatoliy Averbukh]

Checked in.

Comment 47

[sujay]

verified in 6/22 build...I simply disconnected my network cable
from my box and saw the message after 20 seconds.

Comment 48

[Samir Gehani]

*** Bug 48664 has been marked as a duplicate of this bug. ***