Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog

VERIFIED FIXED

Status

SeaMonkey
Preferences
P2
normal
VERIFIED FIXED
18 years ago
14 years ago

People

(Reporter: Simon Fraser, Assigned: Dan M)

Tracking

({crash, platform-parity})

Trunk
PowerPC
Mac System 9.x
crash, platform-parity

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [pdtp2][nsbeta3-][rtm-])

Attachments

(1 attachment)

(Reporter)

Description

18 years ago
In the Smart Browsing prefs panel, click the 'More information' button under the 
Internet Keywords group.

This opens a new browser window, which appears ON TOP OF the prefs dialog. This 
new window cannot be closed, moved or resized.

The real bug here is probably one with window layering, which is danm's.
(Reporter)

Comment 1

18 years ago
nsbeta3, since this there is no easy way out once you get into this situation.
Keywords: nsbeta3
(Reporter)

Comment 2

18 years ago
As an aside: clicking the 'More information' button in a commercial build causes 
a JS error:
JavaScript error: 
 line 0: uncaught exception: [Exception... "Component returned failure code: 
0x80004005 (NS_ERROR_FAILURE) [nsIStringBundle.GetStringFromName]"  nsresult: 
"0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: chrome://communicator/
content/pref/pref-smart_browsing.xul :: moreInfo :: line 137"  data: no]

Comment 3

18 years ago
fyi, (and so I can find them later) blake's fix to implement this button action is bug 50873
and the nsonly version of that bug is bug 52296.

Comment 4

18 years ago
the JS error is caused by bug 52296.  someone needs to add smartBrowsingURL to 
brand.properties in the comm tree.

wrt the other issue...can you suggest a better fix for this?  I don't think 
having dan do some lowlevel window work just to fix this button is worth it.  
How are similar issues like this handled on mac?

Comment 5

18 years ago
Maybe something like what the "Understanding Privacy" button does in the 
Advanced > Cookies pref pane would be best.  Namely, it opens a new parent-
modal dialog with just the info (not all the chrome, etc..)  Would this be 
acceptable?
(Reporter)

Comment 6

18 years ago
Dan *has* to fix the window modality issue at some point, because this is a 
toolkit issue. Whether that gets done by RTM affects the fix for this, I think.
Dan probably needs a separate bug.

Comment 7

18 years ago
OK, but I don't know if he needs to fix it for this release.  And there are 
easier ways to fix this bug, I think.  So on Mac, is it impossible to open a 
new window from prefs without having this problem unless the new window is 
modal?  (Not having a Mac, it's very hard for me to work with this bug).

Comment 8

18 years ago
This window can be minimized, and can be closed using Cmd-W.  Perhaps this
should work perfectly, for completeness sake, but do we really need to bring up
a new non-modal browser window on top of a modal dialog?  For 6.0.0?  I don't
think so. We don't have any such button in the current shipping 4.x, so I think
we can live without it a while longer.  ->ben
Assignee: danm → ben

Comment 9

18 years ago
some notes:
I'd rather not see this yanked. Implementing it was beta3+'d and that's why it's in.
To address some of trudelle's issues, actually we do have this in 4.x. It's a menu item
in the top level 'Help' menu and it goes to the same exact, very useful page.

The underlying, more general toolkit bug here (sfraser mentions 'layering') should probably
be written up as a seperate danm bug for a later milestone/release. I'm not familiar enough
w/the issues at hand to write it up myself or I would.

I think we should concern ourselves here with a serviceable interim fix that addresses the
issue from the user's perspective - namely this window that's hard to get rid of and sits on
top of preferences. Blake has asked nicely for this with every comment.

I was trying to figure what the 'right thing' to do would be and didn't come up with much.
However, I did notice that there is one other place in Prefs that solves the problem:
Edit|Preferences. Advanced|Cookies. There's a button titled 'Understanding Privacy'.
Pressing that opens up it's own neat little window, skirting our current issue.

Maybe something can be learned from that example.

Comment 10

18 years ago
need info. don to work on this
Assignee: ben → don
Whiteboard: [need info]

Comment 11

18 years ago
nav triage team:
renaming the bug to "remove the More Information button" from the Smart 
Browsing prefs dialog in the Internet Keywords section
reassigning to matt
nsbeta3+, P2
Assignee: don → matt
OS: Mac System 8.5 → All
Priority: P3 → P2
Hardware: Macintosh → All
Summary: Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog → Remove Internet keywords 'More infomation' button
Whiteboard: [need info] → [nsbeta3+]

Comment 12

18 years ago
OK.  I already asked a couple of times if the `Understanding Privacy' solution 
was acceptable for this window as well, or if anyone had any ideas for a better 
one.  No one who can make any of the decisions {e.g. managers, PDT} answered me 
or even acknowleged my question.

So, although this could be an easy fix (just do the same thing that 
`Understanding Privacy' does), this will be removed...

Comment 13

18 years ago
Sorry Blake, I would respond if I had a clue!

Comment 14

18 years ago
pdt agrees p2.
Whiteboard: [nsbeta3+] → [nsbeta3+][pdtp2]
Target Milestone: --- → M18
confused here: why is this bug still around when bug 52296 was just recently
fixed?

Comment 16

18 years ago
Sarah: this button opens a new, non-modal window from a modal parent window.  
This causes problems on the mac.  I've asked a couple of times if it could be 
fixed how `Understanding Privacy' in Advanced > Cookies has been done, but 
since no one has cared to answer, this button is now getting useless removed.

Comment 17

18 years ago
I'm not ready to give up. Blake, I never even noticed in my comments from 9/14 that you'd
already hit upon the fix I timidly suggested, therefore I failed to mention it later in navtriage.

You seem to indicate that the proposed fix wouldn't be too hard, how would you rate the regression
risk?

I'm asking that we reconsider fixing this the right way since we know how. Given that everyone
only rates this as a P2 even given the sorry state it's in now then I think the possible benefit
(this works like it should and everyone's happy) greatly outweighs any possible risk (we
regress to some ugly/bad/sorry state) because we're already in that bad state.

In an amazing feat of bug-morphing I'm reverting the summary of this bug and removing
the nsbeta3+ for reconderation of the new(old) proposal. If it turns out I get dinged the
summary should revert to:
"Remove Internet keywords 'More infomation' button".
Summary: Remove Internet keywords 'More infomation' button → Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog
Whiteboard: [nsbeta3+][pdtp2] → [pdtp2]

Comment 18

18 years ago
well, assuming we just do it exactly the same way morse did it with 
understanding privacy, there shouldn't be any possibility or risk of regression.
Summary: Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog → Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog

Comment 19

18 years ago
OK Blake, can you do the fix?  Send me mail if and when you can.  Thanks.

Assignee: matt → blakeross
Keywords: rtm
Whiteboard: [pdtp2] → [pdtp2][nsbeta3-][rtm+]

Comment 20

18 years ago
PDT marking [rtm need info] since patch and code reviews are not yet available.
Whiteboard: [pdtp2][nsbeta3-][rtm+] → [pdtp2][nsbeta3-][rtm need info]

Comment 21

18 years ago
guys, I don't know if I'm going to get to this; I'm pretty busy.  When I get 
some time I'll try, but in the meantime, maybe Steve would like to look at 
this?  Steve, this basically entails doing the exact same thing that you did 
with the Understanding Privacy dialog.
Status: NEW → ASSIGNED
Summary: Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog → Internet keywords 'More infomation' button brings up new browser window in front of the prefs dialog

Comment 22

18 years ago
Yes, this probably would have been very simple for me to do.  I could have used 
the identical trick that I used for the privacy tutorial.

Unfortunately I think that the window of opportunity for getting such a fix 
approved by pdt has just closed.  Look at bug 56048 (same problem) which I just 
squeezed in and see the comment that pdt made as they gave their approval.

Comment 23

18 years ago
Would hiding the button on mac (probably a two-line change) be an acceptable 
rtm solution?

Comment 24

18 years ago
It's okay with me, assuming that we leave this bug open (or open a new one) to 
fix it next time around.

Comment 25

18 years ago
Copying John Gable -- John, is it okay to hide this "More Information" button on 
the Mac OS? This button appears in the Smart Browsing preference panel. It means 
Mac users wouldn't see the Netcenter page that explains keywords. However, 
keywords are explained in the online Help.

Comment 26

18 years ago
Created attachment 17360 [details] [diff] [review]
patch - hide button on mac

Comment 27

18 years ago
I attached a trivial 4-line patch (really 3 lines, since one of it just changes 
one of the lines).  This should work fine, but I don't have a mac to test on to 
confirm.  If someone else could, that'd be great...

Matt: please review and Ben: please approve
a=ben for the branch only. I'd like to find a better solution than just hiding 
the button on the trunk. 

Comment 29

18 years ago
a=matt

Comment 30

18 years ago
rtm+.
Whiteboard: [pdtp2][nsbeta3-][rtm need info] → [pdtp2][nsbeta3-][rtm+]

Comment 31

18 years ago
marking [rtm need info]. Two questions: The bug is marked All platforms, but the
fix seems to be for Mac only. How come? Also, Blake says that he hasn't tested
the fix on the Mac, and neither Matt nor Ben explicitly said they'd tested it.
Who has tested?
Whiteboard: [pdtp2][nsbeta3-][rtm+] → [pdtp2][nsbeta3-][rtm need info]

Comment 32

18 years ago
the nav triage team changed this to All/All on 9/20 because the fix was going 
to be to remove the button from all platforms.  We've since come up with a 
better but equally as small and non-risky fix to just hide the button on mac, 
so at least windows and linux get the benefit of having that extra help.  So 
this is once again mac-only.  Matt/Ben, please test the patch to appease PDT.
Keywords: pp
OS: All → Mac System 9.x
Hardware: All → Macintosh
(Reporter)

Comment 33

18 years ago
I object somewhat strongly to such platform-specific "fixes". Remove the button 
everywhere, or don't remove it at all. This is the start of a slippery slope of 
disabling features on certain platforms; we've been there before, and we should 
not be drifting back into that situation.

Comment 34

18 years ago
Please present a rational reason why it is better that we disable this help on 
all platforms when we could just as easily work around this problem by hiding 
it on Mac.  I don't think mac users are going to be that "shocked" if the 
button is in future releases, but not 6.0.  We have done many such "hack" 
workarounds for the branch.  What's the problem here?

If you want the button just to be removed from all platforms, then give it a 
good reason why and it can be considered.  The patch risk and length is more or 
less equal in either case, so I don't see a reason to do as you suggest.

Comment 35

18 years ago
Still waiting on testing from the reviewers..

Comment 36

18 years ago
Blake's fix is OK with me, and the unfortunate decision to pull this minor UI
element just from the Mac is probably the right thing to do to ship with the
least amount of work.

Ben and Matt, can you confirmed that you tested this on Mac?  If yes, it sounds
like we have all the reviews needed to ship this baby.  If you get no traction,
beat up Don.

Comment 37

18 years ago
And now it's been a week with no traction.

*Beats up don*

Comment 38

18 years ago
chill.  You can beat up Don for so many other things, don't waste it on this.  
Paul kindly agreed to test this tomorrow if he can get his mac building 
working.  Thanks!
Keywords: crash

Comment 39

18 years ago
Well, my commercial build is completely unusable, but my mozilla build works.
Don't see no stinkin' "More Information" button on mac.

Comment 40

18 years ago
The RTM train is leaving the station.  Lots of talk in the bug... but no reviews
or nomination :-/

Comment 41

18 years ago
Jim, Matt and Ben already reviewed / approved.  We've been waiting for testing 
all this time.  But Paul just tested it on his mozilla build, see his 10:56 
comment from today.  Marking rtm+...
Whiteboard: [pdtp2][nsbeta3-][rtm need info] → [pdtp2][nsbeta3-][rtm+]

Comment 42

18 years ago
This bug is in candidate limbo.

Comment 43

18 years ago
rtm-, not stop ship.

Updated

18 years ago
Whiteboard: [pdtp2][nsbeta3-][rtm+] → [pdtp2][nsbeta3-][rtm-]

Comment 44

18 years ago
What?  A crash when clicking a very visible button isn't a stop-ship?  This 
patch has been ready since 10/17, so this is a little disappointing. I'm still 
confused why PDT asked that this 3-line patch be explicitly tested, when they 
don't seem to do that elsewhere.  Then again, it also shouldn't have taken 2 
weeks to get this little patch tested.  Another Mac embarrassment.

Sigh...

This is yours now Dan, I guess.
Assignee: blakeross → danm
Status: ASSIGNED → NEW
(Assignee)

Comment 45

18 years ago
The patch in bug 56677 should solve the general modal window layering problem, by 
forcing sons of modal windows to also be modal. That's my plan for now. Not for 
the Netscape RTM branch, I expect, though that patch is being considered because 
it may address another stop-ship candidate, bug 58417.
Status: NEW → ASSIGNED
Depends on: 56677

Comment 46

18 years ago
We hate rejecting fixes like this, but we're _way_ over the deadline for having
a fully baked build to release.  This was a great fix to get into the build a
couple weeks ago.  It's not a couple weeks ago anymore.
(Assignee)

Comment 47

18 years ago
Fixed by the fix for bug 56677; the new browser window is forced modal to cohabit 
properly with the modal prefs window.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → FIXED
Target Milestone: M18 → M19
vrfy using opt comm *trunk* bits 2000.11.13.08: on winNT and mac, the new
browser window that appears is now modal [gotta close it before you can access
the prefs dialog again]. rather moot on linux, where modality is, um, odd --ie,
the new browser window isn't modal.
Status: RESOLVED → VERIFIED

Comment 49

18 years ago
*** Bug 61381 has been marked as a duplicate of this bug. ***
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.