Closed Bug 87353 Opened 23 years ago Closed 21 years ago

nsIWebNavigation::Reload() using mask flag crashes embedding app

Categories

(Core Graveyard :: Embedding: APIs, defect)

x86
Windows NT
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Future

People

(Reporter: depman1, Assigned: adamlock)

Details

Attachments

(1 file, 1 obsolete file)

reproduced in testembed. /mozilla/embedding/qa/testembed. Using 6/19 embedding
build.
1. Load a uri with the mask flag: 

rv = qaWebNav->LoadURI(NS_ConvertASCIItoUCS2(theUrl).GetUnicode(), 
			nsIWebNavigation::LOAD_FLAGS_MASK);

2. Do a Reload with the mask flag:

   rv = qaWebNav->Reload(LOAD_FLAGS_MASK);

Result: Get assertion: Reload command not updated to load flags! '((aReloadFlags
& 0xf0 == 0)', file ../mozilla/docshell/base/nsDocShell.cpp  line 2169. Crash.

In the debugger, get this call stack:
nsDebug:Assertion()               nsDebug.cpp
nsDocShell::Reload()              nsDocShell.cpp
nsWebBrowser::Reload()            nsDocShell.cpp

Expected: Should reload the previous uri fine.
Quite a few flag combinations may crash embedding. We should put some logic 
into LoadURI and Reload that stops duff combinations or at least asserts on 
them.

The valid combinations are defined in this enum below:

http://lxr.mozilla.org/seamonkey/source/docshell/base/nsDocShell.h#87
David could you apply the patch and see if it catches the strange flag 
combinations?

Potentially the Reload method could be even stricter, rejecting LoadURI 
specific flags.
Adam, this patch did the trick (using yesterday's build). No crash for reloading
with mask flag.
Rick is this bug still relevant?
It looks like this was just a debug ASSERT not an actual crash...
Target Milestone: --- → mozilla0.9.6
Moving out
Target Milestone: mozilla0.9.6 → mozilla1.0
QA Contact: mdunn → depstein
Bugs targeted at mozilla1.0 without the mozilla1.0 keyword moved to mozilla1.0.1 
(you can query for this string to delete spam or retrieve the list of bugs I've 
moved)
Target Milestone: mozilla1.0 → mozilla1.0.1
Changing target milestone to 'Future' since 'mozilla1.0.1' came and went already.
Target Milestone: mozilla1.0.1 → Future
QA Contact: depstein → carosendahl
Attached patch Patch 2Splinter Review
The bug that time forgot! I've updated the patch and run through various link
clicks, normal loads, history loads, reloads and charset reloads and it appears
to function fine.
Attachment #39920 - Attachment is obsolete: true
Comment on attachment 123831 [details] [diff] [review]
Patch 2

Requesting an r/sr on this patch. It asserts and blocks illegal loadtype
combinations.
Attachment #123831 - Flags: superreview?(blizzard)
Attachment #123831 - Flags: review?(radha)
Comment on attachment 123831 [details] [diff] [review]
Patch 2

sr=blizzard
Attachment #123831 - Flags: superreview?(blizzard) → superreview+
Attachment #123831 - Flags: review?(radha) → review+
Fix is checked into trunk
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: