Bug 9577 – Fallback for <object> not rendered if plug-in unavailable

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Nicholas Shanks]

When plug-ins are disabled, <object> elements don't instead display their fallback HTML code.
See above page for an example

[Alexey Proskuryakov]

Duplicate of bug 3742?

[Rob Buis]

Created attachment 9129 [details]
First attempt

The patch fixes the bug for me. I can do testcase(s) if the code gets okayed in the first place :-)
Cheers,

Rob.

[Rob Buis]

Created attachment 9151 [details]
Improved patch

This patch fixes a problem with the one before, m_hasFallbackContent
has to be determined if plugins are disabled.
Cheers,

Rob.

[Rob Buis]

Created attachment 9166 [details]
improved patch

I am not sure whether the previous patch worked 100%, anyway this one is tested ok
against the testcase and has no regressions. Also it fixes bug 7536, since that one is closely
related. I checked the testcases in FF and IE6 and the patch matches that behaviour.
Let me know if I can start doing the testcases, I think the ones from bug 7536 and this one
are an ok start?
Cheers,

Rob.

[Darin Adler]

Comment on attachment 9166 [details]
improved patch

Does Acid2 still pass?

[Rob Buis]

Hi Darin,

(In reply to comment #5)
> (From update of attachment 9166 [details] [edit])
> Does Acid2 still pass?
In my run-webkit-tests runs acid2 works fine. I wonder btw if it takes into
account plugins enabled/disabled? I tried both but didnt spot a difference.
Cheers,

Rob.

[Maciej Stachowiak]

Comment on attachment 9166 [details]
improved patch

r=me

[Rob Buis]

Hi Maciej, Darin,

(In reply to comment #7)
> (From update of attachment 9166 [details] [edit])
> r=me

Nice!
Any advice on the best road to finish the patch? Is it ok
that it may fix two seperate bugs? Does it need a testcase for
both, a combined testcase or are there existing testcases that test
the two bugs?
Cheers,

Rob.

[Darin Adler]

(In reply to comment #8)
> Any advice on the best road to finish the patch? Is it ok
> that it may fix two seperate bugs? Does it need a testcase for
> both, a combined testcase or are there existing testcases that test
> the two bugs?

The more test cases the better. Ideally the test cases independently demonstrate each thing you noticed that was broken in the old code, anything you realize you might have gotten wrong if you weren't such a careful programmer, and anything you're concerned that later hackers might break in the new code you made.

It doesn't really matter how many separate bug reports there are. What's more important is what observable problems there are -- we want to test each of these in case they get screwed up later.

I don't know the status here about which test cases already exist -- you could make a list of what needs to be tested and then check yourself.

[Darin Adler]

Comment on attachment 9166 [details]
improved patch

Clearing the review flag so this doesn't appear in our patches to be commited list. Please set the review flag again when you have a patch with tests that's ready to review and land.

Is the bug still present in TOT?

[David Kilzer (:ddkilzer)]

See also Bug 15202.

[Robert Blaut]

Based on other related and fixed bugs and after checking test url I can suppose that this bug is fixed.