Last Comment Bug 277469 - Repair missing accessibility text equivalents for images
: Repair missing accessibility text equivalents for images
Status: RESOLVED WONTFIX
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: Trunk
: All All
: -- enhancement with 2 votes (vote)
: mozilla1.9alpha1
Assigned To: Mark Pilgrim (inactive)
:
Mentors:
Depends on: 142255 272702 417363
Blocks: htmla11y
  Show dependency treegraph
 
Reported: 2005-01-07 14:06 PST by Aaron Leventhal
Modified: 2016-02-10 10:49 PST (History)
14 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Initial patch, works if accessibity.altrepair or accessibility.getlongdescs pref set. Will go over and clean up before requesting review. (53.28 KB, patch)
2005-01-09 23:24 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Addresses a lot of problems. Getting better. (55.76 KB, patch)
2005-01-10 14:00 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Getting much better. The fallback method often works quite well now, when there is no linked in text to use. (68.08 KB, patch)
2005-01-11 22:11 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
More fixes to fall back method (68.94 KB, patch)
2005-01-12 08:26 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
A log of some of the results generated from some mainstream pages (90.45 KB, text/plain)
2005-01-13 15:05 PST, Aaron Leventhal
no flags Details
Refinements + changes to nsParser::OnStartRequest to use error return from mObserver->OnStopRequest to halt parser (75.31 KB, patch)
2005-01-13 15:11 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Basic handling of XHTML (will get title). (78.10 KB, patch)
2005-01-16 09:49 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Ready for review (78.43 KB, patch)
2005-01-16 20:59 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review
Contains some fixes suggested by pkw, including better i18n for "linksto" string composition (78.71 KB, patch)
2005-01-18 07:56 PST, Aaron Leventhal
bzbarsky: superreview-
Details | Diff | Splinter Review
Not ready for review -- still more to do via bz's comments. Keeping a record as it evolves. (90.28 KB, patch)
2005-01-29 23:02 PST, Aaron Leventhal
no flags Details | Diff | Splinter Review

Description Aaron Leventhal 2005-01-07 14:06:41 PST
When ALT text is missing on an image or image map area, and there is no title,
our current repair strategies are not very good.

For an image link or image map area, we can follow to the linked-to page and
grab the title.
Comment 1 sairuh (rarely reading bugmail) 2005-01-07 14:12:43 PST
aaron, is this problem only specific to Firefox (branch or trunk or both?), or
does it also occur in Seamonkey?
Comment 2 Aaron Leventhal 2005-01-07 14:19:38 PST
The repaired text will be available through the accessible name interfaces for
each accessibility API toolkit.
Comment 3 Benjamin Smedberg [:bsmedberg] 2005-01-08 02:35:02 PST
When do you plan to retrieve the linked page title? Async network loading and
privacy issues both come into play here, as well as trying to convert an
arbitrary network load into usable title information.
Comment 4 Hixie (not reading bugmail) 2005-01-08 06:47:00 PST
Make sure to ONLY do this for images in *links*. The current algorithm was tuned
over a very long time to be the best we could come up with for non-link images.
Comment 5 Aaron Leventhal 2005-01-08 16:59:55 PST
(In reply to comment #4)
> Make sure to ONLY do this for images in *links*. The current algorithm was tuned
> over a very long time to be the best we could come up with for non-link images.

Which code are you talking about? The current impl we have for creating an
accessible name for missing ALT text lives in the accessibility module and just
uses the filename of the image. It's not finely tuned.
Comment 6 Aaron Leventhal 2005-01-08 17:11:17 PST
(In reply to comment #3)
> When do you plan to retrieve the linked page title? 
Several conditions must be present:
1. Accessibility API impl has been initialized by a screen reader or other
assistive technoloy.
2. Pref is turned on (turned off by default for now)
3. Image content with no ALT text, but links to another page, is initialized.

> Async network loading
Is this an issue?

> and privacy issues both come into play here
I'd like to understand this more, but this is still something that 1) will
really only come into play for users who need it, such as screen reader users
and 2) can be turned off. What's the privacy issue, anyway? Cookie related?

> as well as trying to convert an arbitrary network load into usable 
> title information.
It has a fallback repair method if the incoming data is not HTML. The fallback
repair method is similar to the old method of using the image filename, but also
uses the name of the document being pointed to. It tries to choose the better
name from those two.

Comment 7 Aaron Leventhal 2005-01-09 23:24:01 PST
Created attachment 170802 [details] [diff] [review]
Initial patch, works if accessibity.altrepair or accessibility.getlongdescs pref set. Will go over and clean up before requesting review.
Comment 8 Aaron Leventhal 2005-01-09 23:25:29 PST
Benjamin, I just realized you weren't CC'd when I added comment 6.
Comment 9 Benjamin Smedberg [:bsmedberg] 2005-01-10 04:32:44 PST
> > Async network loading
> Is this an issue?

You aren't allowed to block the main thread to do network loading, so yes.

> and 2) can be turned off. What's the privacy issue, anyway? Cookie related?

You are going to need to validate the protocol; you must not try to pre-load
javascript: URIs, because when they are attached to links they frequently
perform one-time actions that should only occur when the user actually chooses
the link. In addition, you need to make sure you are security-checking using
checkLoadURI.
Comment 10 Boris Zbarsky [:bz] (TPAC) 2005-01-10 12:36:54 PST
So... that won't work at all for XHTML, huh?
Comment 11 Aaron Leventhal 2005-01-10 13:59:30 PST
Bz, I'll test/work on the XHTML part in a separate bug, if that's okay. XHTML is
important. But, I'm not actually sure that it won't work as it is.
Comment 12 Aaron Leventhal 2005-01-10 14:00:18 PST
Created attachment 170852 [details] [diff] [review]
Addresses a lot of problems. Getting better.
Comment 13 Boris Zbarsky [:bz] (TPAC) 2005-01-10 14:36:50 PST
Do you really want to be extracting something from 404 responses?

Also, is there a reason the channels aren't being added to the page's loadgroup?
I'd think that leaving the page should cancel those channels, no?
Comment 14 Aaron Leventhal 2005-01-11 03:17:49 PST
(In reply to comment #13)
> Do you really want to be extracting something from 404 responses?
Definitely not. How do I detect them?

> 
> Also, is there a reason the channels aren't being added to the page's loadgroup?
> I'd think that leaving the page should cancel those channels, no?
Leaving the page (closing it) will cancel my channels indirectly, because the
image accessible will be destroyed. Cancelling the load won't yet though. Sounds
like using the loadgroup is good. What other things does the load group
association do besides sharing load cancels?
Comment 15 Hixie (not reading bugmail) 2005-01-11 08:06:34 PST
> Which code are you talking about? The current impl we have for creating an
> accessible name for missing ALT text lives in the accessibility module and just
> uses the filename of the image. It's not finely tuned.

My bad, I thought you meant the text that we made up for missing alt attributes.
Comment 16 Boris Zbarsky [:bz] (TPAC) 2005-01-11 09:52:58 PST
(In reply to comment #14)
> > Do you really want to be extracting something from 404 responses?
> Definitely not. How do I detect them?

http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/http/public/nsIHttpChannel.idl#179

> What other things does the load group association do besides sharing load
> cancels?

Allows the progress on the loads to be tracked by the progress listeners
attached to the page, and allows the loads to block the onload event (unless the
LOAD_BACKGROUND flag is used).
Comment 17 Aaron Leventhal 2005-01-11 16:14:56 PST
(In reply to comment #15)

> My bad, I thought you meant the text that we made up for missing alt attributes.

No, the bug title is misleading. Fixing.
Comment 18 Aaron Leventhal 2005-01-11 16:19:07 PST
The one thing I really don't like is the strange ways I have to kill the
parser+request combo. It's like trying to stop a freight train with an emegency
break. Stuff you don't want still happens afterward.

  mParser->CancelParsingEvents();
  mParser->Terminate();
  request->Cancel(NS_ERROR_HTMLPARSER_STOPPARSING);
  mChannel = nsnull;
  mParser = nsnull;

That's still not good enough. Sometimes my OnStartRequest() still gets called
after that. To avoid a crash I have to keep an "have we died yet" boolean member
variable around, so my OnStartRequest doesn't try to use any dea pointers.
Comment 19 Aaron Leventhal 2005-01-11 17:54:50 PST
(In reply to comment #18)
> That's still not good enough. Sometimes my OnStartRequest() still gets called
> after that. To avoid a crash I have to keep an "have we died yet" boolean member
> variable around, so my OnStartRequest doesn't try to use any dea pointers.

In fact, in some cases the parser itself keeps going, calling things like AddLeaf().

How can I stop the parser and network request at the same time?
Comment 20 Aaron Leventhal 2005-01-11 22:11:23 PST
Created attachment 170998 [details] [diff] [review]
Getting much better. The fallback method often works quite well now, when there is no linked in text to use.

Still to do:
// 1. Fix http://my.netscape.com/index2.psp//
//    With NodeInserted handling its ovewriting its own progress.
//    Can't load 2nd page.
// 2. netscape.com still crashing sometimes
//    0[3d30f0]: ###!!! ASSERTION: This cache entry shouldn't exist already:
'!oldAccessNode', file c:/moz/mozilla/accessible/src/base/nsAccessNode.cpp,
line 451
// 3. AccessibleObjectFromPoint broken
// 4. File separate bug to make it work with XHTML
// 5. File separate bug on Terminate() not working correctly
Comment 21 Aaron Leventhal 2005-01-12 08:26:58 PST
Created attachment 171046 [details] [diff] [review]
More fixes to fall back method
Comment 22 Aaron Leventhal 2005-01-13 15:05:26 PST
Created attachment 171201 [details]
A log of some of the results generated from some mainstream pages

This should give people some insight into the repairs that we'll be able to do
with this fix.

Note, some of these images already had ALT text, but I'm logging the repairs
that would have happened. When the log says * Link repair by title, it means
that the fallback text is not necessary, but is calculated anyway to help
refine the heuristic.
Comment 23 Aaron Leventhal 2005-01-13 15:11:31 PST
Created attachment 171203 [details] [diff] [review]
Refinements + changes to nsParser::OnStartRequest to use error return from mObserver->OnStopRequest to halt parser
Comment 24 Aaron Leventhal 2005-01-16 09:49:42 PST
Created attachment 171419 [details] [diff] [review]
Basic handling of XHTML (will get title).

Last remaining problem is to figure out why cancelling a load screws up future
loads.
Comment 25 Boris Zbarsky [:bz] (TPAC) 2005-01-16 10:33:04 PST
Note that nsIChannel inherits from nsIRequest, so QIing mChannel to nsIRequest
is not needed....
Comment 26 Aaron Leventhal 2005-01-16 20:59:19 PST
Created attachment 171467 [details] [diff] [review]
Ready for review

Boris, would you be willing to look at this since you have been a little bit
already?
Comment 27 Boris Zbarsky [:bz] (TPAC) 2005-01-16 21:23:44 PST
Er... I can try, but a real review (instead of just looking at a random spot and
commenting if I see something) will take some time... :(
Comment 28 Aaron Leventhal 2005-01-17 09:05:41 PST
(In reply to comment #27)
> Er... I can try, but a real review (instead of just looking at a random spot and
> commenting if I see something) will take some time... :(

I understand. When do you think you'll have time for it? The title grabbing code
will be off by default at first.
Comment 29 Boris Zbarsky [:bz] (TPAC) 2005-01-17 09:33:38 PST
I'll try to look at it this coming weekend...
Comment 30 Aaron Leventhal 2005-01-18 07:56:20 PST
Created attachment 171640 [details] [diff] [review]
Contains some fixes suggested by pkw, including better i18n for "linksto" string composition
Comment 31 Louie Zhao 2005-01-20 02:04:30 PST
Before going through the patch, I have one question for this solution: If a page
contains many linkable images which have no ALT text, mozilla will try to load
all these linked pages (am I right?). How will this impact the performance ?
Comment 32 Boris Zbarsky [:bz] (TPAC) 2005-01-29 14:24:06 PST
Comment on attachment 171640 [details] [diff] [review]
Contains some fixes suggested by pkw, including better i18n for "linksto" string composition

>Index: accessible/src/base/nsAccessible.cpp

>+  if (inputContent || objectContent) {

This branch is wrong for <object>.  <object> doesn't use <alt>, and if an
<object> cannot be rendered (with the exception of sized image objects in
quirks mode) it will show its children as  the alternate content.  So for
<object> the right check is to check whether it has children.

Also, why are we treating <img> and <input type="image"> differently?

>Index: accessible/src/base/nsBaseWidgetAccessible.cpp
>+NS_IMETHODIMP nsBaseImageAccessible::Init()

Is this expecting to be instantiated for only certain types of nodes?  If so,
please put preconditions here asserting that.  I can't tell whether this code
is right as things stand.

>+  // Split the new URI into a left and right part
>+  // (assume we're parsing it out right

Please, please do not do this.	There are nsIURL methods that will get you what
you want here, no?  Don't do your own URI parsing.

>+void nsBaseImageAccessible::GetLinkToText(nsAString& aResult,

I didn't really chek the logic here very carefully...

>+nsAString& nsBaseImageAccessible::CoreOfString(nsString& aText)
>+  while (iterEnd != iterStart) {
>+    if (*iterEnd == '/') {

So if your string ended with a char that was not '/', why is it ok to to
*iterEnd here?

>+      vanillaEnd.advance(-NS_STATIC_CAST(PRInt32, vanillaPageName.Length()));

What if this pushes vanillaEnd to before iterStart?

>+already_AddRefed<nsIURI> nsBaseImageAccessible::GetURIFromOwnerDoc()

>+  webNav->GetCurrentURI(&baseURI);

So are we getting a base URI or a current URI?

If you want a base URI for an image or link or something, you should be getting
it off the nsIContent or nsIDOM3Node for that element.	Using the webnavigation
URI there is wrong.

>+nsBaseImageAccessible::NewTextEquivFromURI(nsAString& aLinkURI,
>+  if (!securityManager ||
>+      securityManager->CheckLoadURI(baseURI, uri, kCheckFlags) == NS_ERROR_DOM_BAD_URI) {
>+    return nsnull;
>+  }

Please treat all error returns from CheckLoadURI as equally fatal.

Should you be doing an nsIContentPolicy check here?

>Index: accessible/src/base/nsBaseWidgetAccessible.h

>+  static nsTextEquivFromURI* NewTextEquivFromURI(nsAString& 
aLinkURI,

This needs to document that it allocates the object with new (and that it may
return null if the allocation fails).

>Index: accessible/src/base/nsTextEquivFromURI.cpp
>+    // XXX Is there a way to wait on setting up the parser until after
>+    //     we know the content type?

Yes, you could implement nsIStreamListener yourself, and set up the parser in
OnStartRequest, then just proxy over the notifications.

>+    mParser->SetContentSink(mSink);

You're passing in an XPCOM object with a refcount of 0?  Don't do that, please.

>+    mParser->Parse(aURI, mSink, PR_FALSE, nsnull, eDTDMode_quirks);

This is wrong.	It'll get the wrong text out of some documents...  Why,
exactly, is this being done?

>+    mChannel->AsyncOpen(streamListener, nsnull);

You want to set the priority before this call, no?

>+void nsTextEquivFromURI::Finish(PRBool aSuccess)
>+  mSink->Kill();
>+  mChannel = nsnull;
>+  mParser = nsnull;

What about nulling out mSink?

>+  if (!gParserService) {
>+    nsCOMPtr<nsIParserService> parserService =
>+      do_GetService("@mozilla.org/parser/parser-service;1");
>+    gParserService = parserService;

This is wrong.	gParserService needs to hold a ref to the service, and you need
to release it at shutdown.  As written, this code will crash if someone
restarts XPCOM, eg.

>+    NS_ASSERTION(gParserService, "Could not retrieve parser service");  

That can happen (either because there is no more memory, or for other reasons).
 It needs to be handled, not just asserted on.

>+ nsTextEquivContentSink::OnStartRequest(nsIRequest *aRequest, nsISupports *aContext)

>+    httpChannel->GetRequestSucceeded(&requestSucceeded);
>+    if (!requestSucceeded) {
>+      return NS_BINDING_ABORTED;
>+    }

No need to call Finish()?

>+ !contentType.Equals("application/x-unknown-content-type")) { // Often HTML

This should NOT be happening, for HTTP requests.  If it is, please point me to
a URL triggering this.	I don't know whether you want to parse random files
that we couldn't figure out the type for as HTML.  That sounds like a good way
to confuse the parser (consider image files).

>+NS_IMETHODIMP nsTextEquivContentSink::OpenContainer(const nsIParserNode& aNode)

>+    if (aTag >= eHTMLTag_h1 && aTag <= eHTMLTag_h6) {

I'd enumerate explicitly instead of assuming things about eHTMLTag ordering.

>+NS_IMETHODIMP nsTextEquivContentSink::CloseContainer(const nsHTMLTag aTag)
>+  if (aTag >= eHTMLTag_h1 && aTag <= eHTMLTag_h6 && 

Same.

>+PRBool nsTextEquivContentSink::MatchesXHTMLTag(const PRUnichar *aExpatName, 

Er... how would this be called, exactly?  You explicitly prevent expat ever
being invoked...

>+  const char* xhtmlNameSpace = "application/xml+xhtml";
>+  const PRUint32 xhtmlNameSpaceLen = 28;

NS_ARRAY_LENGTH, please?  That would mean making xhtmlNameSpace a char[], which
is fine.  Any  reason these are not static?

>Index: accessible/src/base/nsTextEquivFromURI.h
>+  nsTextEquivContentSink *mSink; // Weak ref, avoid circular references

What's the cycle you're avoiding, exactly?  I don't see one here...

>Index: accessible/src/html/nsHTMLImageAccessible.cpp
>+NS_IMETHODIMP nsHTMLImageAccessible::Init()

Again, what nodes can this be used for?  It should assert accordingly.

>+void nsHTMLImageAccessible::GetFallbackName(nsAString &aText)

>+  nsCOMPtr<nsIURI> baseURI = GetURIFromOwnerDoc();

Again, this gives the wrong base URI.

>Index: layout/base/nsIPresShell.h

>+class nsIAccessibilityService;

No ifdef needed?

>Index: layout/base/nsPresShell.cpp
>@@ -5857,10 +5880,19 @@ PresShell::HandleEventInternal(nsEvent* 
>+      static PRBool accessPrefsInited;

Why are we initing these in HandleEventInternal, exactly?  Why are we not
observing pref changes?  This is wrong for profile switches, for example.

>Index: layout/generic/nsImageFrame.cpp

>+#ifdef ACCESSIBILITY
>+  if (aPresContext->PresShell()->GetRepairingTextEquivalents() &&
>+      (!mContent->HasAttr(kNameSpaceID_None, nsHTMLAtoms::alt) ||
>+      mContent->HasAttr(kNameSpaceID_None, nsHTMLAtoms::usemap)) ||
>+      (aPresContext->PresShell()->GetRetrievingLongDescriptions() &&
>+      mContent->HasAttr(kNameSpaceID_None, nsHTMLAtoms::longdesc))) {

This is absolutely wrong for an nsImageFrame that's a frame for an <object>.

Perhaps some code in nsCSSFrameConstructor::CantRenderReplacedElement should be
factored out into this code?

Also, does the accessible hold a pointer to the frame or the content?  What
does it do if the frame is destroyed?

I'll wait for an r= before doing the next round of review....
Comment 33 Aaron Leventhal 2005-01-29 21:39:50 PST
Thanks for all the comments. Making a lot of changes based on them. Some questions:

>Index: accessible/src/base/nsAccessible.cpp

> Also, why are we treating <img> and <input type="image"> differently?
We don't want to try and follow a link around an <input> for alt text repair.
We only do that when there is an img inside of a link.

>>+  // Split the new URI into a left and right part
>>+  // (assume we're parsing it out right
> Please, please do not do this.  There are nsIURL methods that will get you what
> you want here, no?  Don't do your own URI parsing.
I agree there should be an nsIURI method for this, but there isn't.
I grabbed my parsing code from nsDocShell.cpp, which also needs this.

>>+nsAString& nsBaseImageAccessible::CoreOfString(nsString& aText)
>>+  while (iterEnd != iterStart) {
>>+    if (*iterEnd == '/') {
> So if your string ended with a char that was not '/', why is it ok to to
>*iterEnd here?
I don't understnad that question. What's wrong with *iterEnd?

More replies later. 
Comment 34 Aaron Leventhal 2005-01-29 22:31:49 PST
>>+ !contentType.Equals("application/x-unknown-content-type")) { // Often HTML

> This should NOT be happening, for HTTP requests.  If it is, please point me to
> a URL triggering this.  I don't know whether you want to parse random files
> that we couldn't figure out the type for as HTML.  That sounds like a good way
> to confuse the parser (consider image files).

We never parse the wrong file becuase I hvae a second check in WillBuildModel()
where I look at dtdType, which is always correct at that point. However, in
OnStartRequest() the content type  is still sometimes unknown.

I'll post a URI where it happening later.

> Er... how would this be called, exactly?  You explicitly prevent expat ever
> being invoked...
Where do I do that? Where I ask for eDTDMode_quirks? That's not preventing expat
 from being invoked. I've tested and used my nsIExpatSink impl already. I'd
actually prefer if I didn't need an nsIExpatSink impl and it would always use 
the html content sink for XHTML. That would make the code simpler.

> Also, does the accessible hold a pointer to the frame or the content?  What
> does it do if the frame is destroyed?
All accessibles hold onto nsCOMPtr<nsIDOMNode> mDOMNode
We listen for mutation events and pay attention to what documents are destroyed.
A Shutdown() method for an accessible any time it's node is being destroyed, so
that references like mDOMNode can be destroyed without destroying the entire
accessible. It needs to remain in memory in case the out-of-process client still
calls methods on it. However, it will return failures codes for methods called
on it at that point.

The only kind of accessible that holds onto a frame is an nsHTMLTextAccessible,
created for text nodes
nsHTMLTextAccessible.h:  nsIFrame *mFrame; // Only valid if node is not shut
down (mWeakShell != null)
Could that frame get destroyed without a mutation event occuring or the document
being destroyed?  That would be a problem at this point.
Comment 35 Aaron Leventhal 2005-01-29 22:49:05 PST
  >+ mParser->Parse(aURI, mSink, PR_FALSE, nsnull, eDTDMode_quirks);
  > This is wrong. It'll get the wrong text out of some documents... Why,
exactly, is this being done?

Which part is wrong, the eDTDMode_quirks or the Parse() call itself?

I don't know exactly what calling sequence I should be using to start things. Do
I not need this if I follow your advice about "you could implement
nsIStreamListener yourself, and set up the parser in OnStartRequest, then just
proxy over the notifications."
Comment 36 Aaron Leventhal 2005-01-29 23:02:45 PST
Created attachment 172858 [details] [diff] [review]
Not ready for review -- still more to do via bz's comments. Keeping a record as it evolves.
Comment 37 Boris Zbarsky [:bz] (TPAC) 2005-01-29 23:15:52 PST
> I agree there should be an nsIURI method for this, but there isn't.

My comment said nsIURL, not nsIURI, no?  The docshell code is rather broken; see
the bug I have to remove it.

> I don't understnad that question. What's wrong with *iterEnd?

If iterEnd points past the end of the string (which it does in the case I
mentioned), it's something that's not in your string.

> where I look at dtdType, which is always correct at that point.

That depends on the parser getting the dtdType from the data, which may look
nothing like HTML...

> and it would always use the html content sink for XHTML.

The HTML sink can't parse XHTML correctly; it'd give bogus data back. My
question was based on your comment, which claimed that you didn't have to
implement nsIExpatSink, since you were passing a quirks mode.

> Could that frame get destroyed without a mutation event occuring or the
> document being destroyed? 

Absolutely.  Style changes could do it, for example, and not all of those are
accompanied by mutation events (preference changes come to mind, as does
enabling/disabling of stylesheets by the document).  I'm assuming you watch for
mutation events on attributes; if you don't then inline style changes will also
kill off frames without you noticing.

> Which part is wrong, the eDTDMode_quirks or the Parse() call itself?

The former.  That will parse documents wrong and give you bogus data in some cases.

> I don't know exactly what calling sequence I should be using to start things.

In general, this is fine.  You should be letting the parser autodetect the
parsemode, though.
Comment 38 Aaron Leventhal 2005-01-30 08:43:45 PST
Boris, holding onto the text frames indeed sees like a problem.
Is there any listener I can attach to listen for frames that are destroyed?

It's just an optimization for text frames that I have, because
GetPrimaryFrameFor() does a lot more calculation for text frames than what I do.
Doing GetPFF on text frames would increase the size of the primary frame map
since those are normally not stored there. I already havee the text frames as I
iterate through, and cahce those on the accessibles. I'd like to keep the
optimzation if possible.
Comment 39 Boris Zbarsky [:bz] (TPAC) 2005-01-30 09:14:01 PST
You could set the NS_FRAME_EXTERNAL_REFERENCE bit on the framestate and add a
hook to PresShell::ClearFrameRefs that's #ifdef ACCESSIBILITY.
Comment 40 Aaron Leventhal 2005-01-31 03:00:16 PST
> Should you be doing an nsIContentPolicy check here?

Do I need to provide my own implementation of nsIContentPolicy to prevent it
from loading anything other than HTML? Or should the mime type check be enough?
Comment 41 Aaron Leventhal 2005-01-31 03:18:44 PST
(In reply to comment #39)
> You could set the NS_FRAME_EXTERNAL_REFERENCE bit on the framestate and add a
> hook to PresShell::ClearFrameRefs that's #ifdef ACCESSIBILITY.

Filed bug 280498 on that issue. It's been wrong for a while, but is really
separate from this bug since that problem is not an image accessible problem.
Comment 42 Aaron Leventhal 2005-01-31 04:23:57 PST
(In reply to comment #32)
> Why are we initing these in HandleEventInternal, exactly?  Why are we not
> observing pref changes?  This is wrong for profile switches, for example.

Eventually the pref will go away and we will always use this code.
This is just some quick code so that testers can turn it on while it's still
experimental.

What gets shut down when the profile is switched? There is no XPCOM or sevices
shutdown or anything right?
Comment 43 Boris Zbarsky [:bz] (TPAC) 2005-01-31 07:56:42 PST
> Do I need to provide my own implementation of nsIContentPolicy

Probably not, but I was wondering whether this code should load things that are
blocked by whatever content policy modules are loaded...  I'm not sure, hence
the question.

> What gets shut down when the profile is switched?

I'm not completely sure, actually.  Certainly preferences get reset....  I don't
think anything is really shut down, though.

> There is no XPCOM or sevices shutdown or anything right?

Right.  That can happen for other reasons, though.
Comment 44 timeless 2005-01-31 10:32:09 PST
iirc networking is supposed to go away temporarily :)
Comment 45 Aaron Leventhal 2007-04-12 21:34:48 PDT
It's going to be hard to merge this patch to trunk, but if get the chance to, have text equiv repair would be great. I suggest we expose an object attribute called "name-guess".
Comment 46 Aaron Leventhal 2007-12-06 10:15:18 PST
The name-guess attribute would just be "true" if the accessible name was from the repair operation.
Comment 47 Aaron Leventhal 2008-04-12 15:43:30 PDT
When a PNG image is used we should look for the accessible name as a title inside the file:
See http://www.vias.org/pngguide/chapter11_04.html
In this case it's not a guess -- the author of the .png put the text directly in the image. I don't know how common that is.
Comment 48 David Bolter [:davidb] 2016-02-10 10:49:06 PST
Last activity was 2009-08-22. For practical reasons I'm closing as WONTFIX.

Note You need to log in before you can comment on or make changes to this bug.