21456 - ALT attribute for APPLET not working

Status: VERIFIED FIXED

(Core :: Layout, defect, P3)

Description

[Chris Petersen]

Build: Apprunner
Version: 1999121008
Platforms: ALL

Expected Results: The ALT value should be displayed if the Applet can't be
rendered.

What I got: On the Mac, I geta blank page with no alttext; On Windows and Linux,
I get a box with the words "applet". The alt text is not displayed.


Steps to reproduce:

1) In Apprunner, turn off Java in the Advanced section of Preferences. Quit and
relaunch Apprunner.

2) Load the following test case:
http://slip/projects/marvin/html/applet_alt.html

3) The test cases loads but the ALT test is not displayed.

Comment 1

[rickg]

Andrei -- please take a look.

Comment 2

[Antti Näyhä]

Attachment: Minimized test case

Comment 3

[Antti Näyhä]

Please keep your test cases public in the future. :-)
For now, I'm attaching a minimized test case which demonstrates the problem.

Actual results on my Dec 19, 1999 Installer build (Windows NT4, no Java
installed):
  Nothing.

Expected results:
  The words "Alternative text" - since the applet cannot be started.

Comment 4

[ekrock's old account (dead)]

Bulk moving [testcase] code to new testcase keyword. Sorry for the spam!

Comment 5

[leger]

Putting on PDT- radar for beta1

Comment 6

[Chris Petersen]

Added keyword nsbeta2.

Comment 7

[leger]

Putting on [nsbeta2-] radar. 

Comment 8

[ekrock's old account (dead)]

Reassigning to George, who's kindly volunteered to take this on. Thanks George!

Comment 9

[drapeau]

Accepting this bug.

Comment 10

[edburns]

I'll take this.

Comment 11

[edburns]

I Accept.

This is the text I sent to waterson, evaughn and hyatt:

Hello,  I have bug 21456 which asks me to make it so APPLETS that specify an 
ALT attribute display the value of the ALT attribute where the applet would 
be.  I looked in to doing this and found myself in

nsCSSFrameConstructor.cpp::CantRenderReplacedElement()

at the part where we say:

else if ((nsHTMLAtoms::object == tag.get()) ||
             (nsHTMLAtoms::embed == tag.get()) ||
             (nsHTMLAtoms::applet == tag.get())) {

    // It's an OBJECT, EMBED, or APPLET, so we should display the contents
    // instead

I'm not familiar with this part of the product at all and I would like some 
guidance on how to proceed.  I think I'm in the right neighborhood.  I think I 
might have to do something with GetAlternateTextFor, but I'm not sure what.  
Can you please help me?

Thanks,

Ed

Comment 12

[edburns]

Attachment: Fix for this bug.

Comment 13

[edburns]

Attachment: Checkin comment for this bug fix, explains the patch.

Comment 14

[edburns]

Attachment: Testcase for this bugfix.

Comment 15

[edburns]

I have just attached a patch, an explanation of the patch, and a testcase for
the patch.  Can someone please review this?

Thanks,

Ed

Comment 16

[edburns]

Attachment: unified diff patch of fix for this bug.

Comment 17

[Chris Waterson]

Your logic is broken. For example,

  <applet alt="whee"><img src="blah.gif"></applet>

Will display "whee", not the image. I think what you probably should do is go 
ahead and construct the frames based on the child content, and then if you 
detect that that yields nothing worth displaying, you should discard them, and 
create a single text frame based on the "alt" text.

Comment 18

[Chris Waterson]

cc vidur & dbaron, who may also have an opinion on the right thing to do 
here...

Comment 19

[edburns]

I hate to throw the book at you, but according to O'Reilly's _HTML, The
Definitive Guide_ section 5.6.5.8:

"Browsers that support the <applet> tag ignore the html content inside.  (Use
the alt attribute to notify applet enabled browser users when the applet doesn't
display for some reason.)"

Therefore, since mozilla supports the applet tag, we shouldn't worry about
content inside.  

Chris, what do you think?

Comment 20

[Chris Waterson]

Ow! Just be sure to pick the right book. Re-read:

  http://www.w3.org/TR/html4/struct/objects.html

Specifically,

  http://www.w3.org/TR/html4/struct/objects.html#h-13.3.1

"A user agent must interpret an OBJECT element according to the following 
precedence rules:

1. The user agent must first try to render the object. It should not render the 
element's contents, but it must examine them in case the element contains any 
direct children that are PARAM elements (see object initialization) or MAP 
elements (see client-side image maps). 

2. If the user agent is not able to render the object for whatever reason 
(configured not to, lack of resources, wrong architecture, etc.), it must try to 
render its contents."

With respect to applets, the spec is clear.

  http://www.w3.org/TR/html4/struct/objects.html#h-13.4

"The content of the APPLET acts as alternate information for user agents that 
don't support this element or are currently configured not to support applets."

Immediately below this text, there are two examples that illustrate how

  <applet>foobar</applet>

is equivalent to

  <object>foobar</object>

Since the spec refers to rendering the "contents" of the <object> tag (and, 
transitively, the <applet> tag), I think that we have to presume that arbitrary 
content is allowed to be contained within an <applet> tag. Furthermore, since 
the spec treats <applet> as a deprecated shorthand for <object>, I think that we 
need to apply the rules appropriate for <object> to <applet> as well. I agree 
that there is some grey area here with the precedence of the "alt" attribute 
with respect to text contained within the tags; however, in your checkin comment 
(attachment #3 [details] [diff] [review]), you seem to have agreed that preferring child content over 
attribute text is the correct thing to do.

So I'll stand by my assertion that the right way to approach the problem is to:

1. build the content frames
2. inspect them to see if they've built only whitespace
3. if so, build a single text frame from the "alt" text.

Furthermore, I believe that said implementation would be significantly simpler 
than what you've proposed.

Comment 21

[edburns]

Attachment: Fix for this bug; second iteration.

Comment 22

[Chris Waterson]

You really need to create the frames here, IMO. Although it's degenerate, some 
bonehead could have a style rule that makes "param" be a "display: inline" or 
something. Or an <img> that's "display: none".

Comment 23

[edburns]

Attachment #6 [details], id=11384 contains a fix that implements different behavior, read
on.

waterson recommends:

> 1. build the content frames
> 2. inspect them to see if they've built only whitespace
> 3. if so, build a single text frame from the "alt" text.

Attachment #6 [details] implements this:

1. Inspect the nsIContent for which CantRenderReplacedElement was called.

   If any of the children of this content are NOT (whitespace text nodes
   OR param elements OR map elements) the child content will be rendered.

2. If step 1 determines that the child content should not be rendered
   see if there is an ALT attribute with a value that can be rendered.

3. If so, done, else fall back on the old behavior of trying to render whatever
   is there

I feel this is simpler than building the frames, then inspecting them.  Why take
up the time to build something if you don't need it?

Comment 24

[Chris Waterson]

Without building the frames, you can't tell if you actually have something to 
display or not. If there's nothing to display, no frames will be built. 
(Well, that's not *quite* true, but, the frames that do get built will be 
minimal.)

Looking at the content nodes is simply not sufficient.

Comment 25

[edburns]

waterson: ok, so I build the frames.  Now I have an nsIFrame * that is the root 
of a subtree.  I assume that everything in this subtree is displayable, right?  
That is, the frames that get build will have nothing but displayable content.  
I want to determine if there is any non-whitespace content in that subtree.

Here's my sketch for the nsIFrame traversal.  This will be recursive in the 
end, but I express it iteratively here for simplicity:

for each nsIFrame in the subtree {
  - call GetContent to get an nsIContent for this frame

  - if the nsIContent is displayable, fall out of the recursion with a 
    FOUND_CONTENT reason
}

If the inspection completed without FOUND_CONTENT, then try to render the alt 
tag.  

My question is, what is the easiest way to tell if an nsIContent is displayable?

Comment 26

[Chris Waterson]

I think your code would look something like this:

static PRBool
HasDisplayableFrames(nsIFrame* aFrame)
{
  // Returns 'true' if there are frames that could be displayed in the
  // frame list.
  while (aFrame) {
    // If it's not a text frame, then assume that it's displayable.
    nsCOMPtr<nsIAtom> frameType;
    aFrame->GetFrameType(getter_AddRefs(frameType));
    if (frameType != nsLayoutAtoms::textFrame)
      return PR_TRUE;

    // Get the text content...
    nsCOMPtr<nsIContent> content;
    aFrame->GetContent(getter_AddRefs(content));

    nsCOMPtr<nsITextContent> text = do_QueryInterface(content);
    NS_ASSERTION(text != nsnull, "oops, not an nsITextContent");
    if (! text)
      return PR_TRUE;

    // Is it only whitespace?
    PRBool onlyWhitespace;
    text->IsOnlyWhitespace(&onlyWhitespace);

    // If not, then we have displayable content here.
    if (! onlyWhitespace)
      return PR_TRUE;

    // Otherwise, on to the next frame...
    aFrame->GetNextSibling(&aFrame);
  }

  // If we get here, then we've iterated through all the frames, and
  // every one is a text frame containing whitespace. There is nothing
  // to diplay.
  return PR_FALSE;
}

Comment 27

[edburns]

Attachment: Fix for this bug; third iteration.

Comment 28

[Chris Waterson]

Attachment: less invasive change

Comment 29

[Chris Waterson]

What do you think?

Comment 30

[Chris Waterson]

I think that there is further work to be done here: the minimized test case left
off the "CODE" attribute. When I add the "CODE" attribute back, I get the
"plugin downloader plugin"; e.g.,

  <applet code="some.class">oops, this'll never be displayed</applet>

Maybe you could poke around nsObjectFrame.cpp and see why this is the case?

Comment 31

[edburns]

Chris, I like this change better than mine.  I'm sorry it's taken so long to 
get approval on this.

Comment 32

[Chris Waterson]

Patch checked in. Adding new test case, which still needs work, probably in 
nsObjectFrame.cpp.

Comment 33

[Chris Waterson]

Attachment: Original Test Case

Comment 34

[edburns]

This is probably another bug, and not an nsbeta2 one.

Still building, will investigate.

Comment 35

[Chris Waterson]

Frankly, the "other bug" is worse! How many <applet> tags without a "code=" 
attribute are there?

Comment 36

[edburns]

Chris, you're certain you don't want to display ALT values for OBJECT tags?

Comment 37

[Chris Waterson]

From 13.8, "The alt attribute must be specified for the IMG and AREA elements. 
It is optional for the INPUT and APPLET elements." There's nothing in there 
about the <object> tag.

Comment 38

[edburns]

Chris, your testcase attachid=11550 has the ALT value floating between <applet> 
and </applet>. When I move the ALT inside the <applet> tag, as is correct, it 
displays correctly.  What's the bug?

Comment 39

[edburns]

Re <object>: Fine and dandy.

Comment 40

[edburns]

Here is what I think the correct behavior should be:

If Java is enabled via prefs:
  If Java is installed, display the applet.
  If Java is not installed display the plugin downloader plugin

If Java is disabled via prefs:
  Display the INLINE TEXT if present, and the ALT text if no INLINE text is 
  present.

This is what is currently implemented.  Please correct me if you have a 
different understanding of what correct behavior is here.

Comment 41

[edburns]

Chris, can I mark this fixed?

Ed

Comment 42

[Chris Waterson]

Ah, ok. I get it. Sounds good.

Comment 43

[edburns]

Marking fixed.

Comment 44

[Chris Petersen]

Fixed in the July 27th build.

Comment 45

[Heikki Toivonen (remove -bugzilla when emailing directly)]

SPAM. HTML Element component deprecated, changing component to Layout. See bug
88132 for details.