[Webkit-unassigned] [Bug 26667] Assertion failure in -[WebHTMLView _handleStyleKeyEquivalent:]

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26667





--- Comment #9 from opendarwin at lapcatsoftware.com  2011-03-30 19:42:25 PST ---
(In reply to comment #8)
> (In reply to comment #7)
> > > I suggest instead changing the assertion.
> > > 
> > >     ASSERT(_private->closed || [self _webView]);
> > 
> > I don't believe that would be a good fix, because while it trivially solves the assertion failure, it makes the method logic flawed. For example:
> > 
> >     if (![[[self _webView] preferences] respectStandardStyleKeyEquivalents])
> >         return NO;
> > 
> > The conditional is accidentally satisfied because [self _webView] is nil, regardless of the preference setting.
> 
> While it may bother you to run code like that that seems to do the right thing for the wrong reason, there is no reason to add code paths to handle the closed case. The code will behave harmlessly when there is no web view and in every other respect as well. We don’t need to add additional code to get the correct behavior in this odd edge case, because everything works fine.
> 
> The only thing causing trouble is the assertion.
> 
> Adding a closed check in this one method will create the false notion that we need to add it elsewhere too, and there is no evidence to suggest that is the case.

It does bother me, and I refuse to write incorrect, fragile code like that.

Frankly, as I was composing my patch, I did get the impression that other methods in this class probably needed a closed check as well. I suspect that the absence of checks is simply the result of lazy, careless coding.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.