Closed Bug 51339 Opened 24 years ago Closed 23 years ago

Image Maps not working in XML files

Categories

(Core :: XML, defect, P3)

Product:
Core
Component:
XML
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9

People

(Reporter: kinger, Assigned: hjtoi-bugzilla)

References

(
URL
)

Details

(Keywords: xhtml, Whiteboard: [fixinhand])

Attachments

(9 files)

simple example of problem
284 bytes, text/html
Details
image for test
391 bytes, image/gif
Details
Alternative fix 1
16.59 KB, patch
Details | Diff | Splinter Review
Alternative fix 2
8.57 KB, patch
Details | Diff | Splinter Review
HTML testcase, evil duplicate names
281 bytes, text/html
Details
XML testcase, evil duplicate names
372 bytes, text/xml
Details
Proposed fix
21.72 KB, patch
Details | Diff | Splinter Review
makemapstresstest.pl
1.03 KB, text/plain
Details
Proposed fix 2
17.07 KB, patch
Details | Diff | Splinter Review 
Build ID: 2000090110
Platform: WinNT4

Image Maps are not functioning in XML documents using the HTML namespace.  The 
problem is not related to the namespace as other HTML elements are working.  The 
URL listed on this bug contains an example.

What is happening is that the whole image is "hot", i.e. cursor changes to a 
link, as opposed to just the area specified.  Also, clicking does nothing.

I added special styles for the elements in the document stylesheet (story.css).  
These are replicated from 'html.css'.

I'd appreciate someone having a look into this, or pointing me in the right 
direction.
This occurs on all platforms. Tested on Mac, Linux, and Win 98 with Sept 5th 
builds.

Created a simple test case to show the same problem.


    
    

    
  

      
      
      
      

        Attached image
          
        image for test
        — 
      

    


  

    
    

    
  
Re-assigning 5 bugs to Heikki from Clayton's bug list.  Please triage.  Thanks!
Assignee: clayton → heikki

    
    

    
  
The reason image maps do not work in XML is because we haven't implemented them.  
I'd guess most HTML elements wouldn't work in XML right now. It might be rather 
simple to implemented them, though, so if anyone reading this wants to give it a 
try I'd say go ahead.

I do not think this is that important for Netscape beta 3, which is why I think 
Netscape personnel will not work on this until 6.0 is out of the door.

Unless someone gets ahead of me I am going to accept this bug, and Future it, 
meaning I will get to it sometime after 6.0.

This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration.
Status: NEW → ASSIGNED
Component: Layout → XML
OS: Windows NT → All
Hardware: PC → All
Target Milestone: --- → Future

    
    

    
  
Heikki, you say it may be "rather simple", so if you give me some details and 
point me in the right direction, I will look into doing this.  We may be able to 
tap into or duplicate some of the HTML implementation, correct ??

    
    

    
  
All HTML namespace elements that work in XML are actually HTML elements 
(codewise). I think we recognize these in the XML content sink. So if I wanted 
to implement image maps in XML I would first look at nsXMLContentSink, see how 
other HTML elements are created and then make sure the elements needed for image 
maps are created as well. It might be as simple as adding to some if-then/switch 
structure a new case so it will create the elements needed by image maps. 
Currently at least IMG and A elements work.

    
    

    
  
Are there plans to refactor some of the content sink code to fix this type of 
problem?

    
    

    
  
It all seems to be happening here:
http://lxr.mozilla.org/seamonkey/source/layout/xml/document/src/nsXMLContentSink
.cpp#574
Any further guidance?
Target Milestone: Future → mozilla0.9

    
    

    
  
Nominating for beta1. Map images are commonly used and should function in XHTML.
Keywords: nsbeta1

    
    

    
  
Moving one milestone further.
Target Milestone: mozilla0.9 → mozilla0.9.1

    
    

    
  
Since the introduction of the new milestone, I can move this back to 0.9.
Target Milestone: mozilla0.9.1 → mozilla0.9

    
    

    
  
I finally found out why this does not work. nsImageMap objects (which make up
most of the image map functionality) are stored in the HTML document! Is that
twisted or what?

Now the simplest fix would be to add that stuff to XML document, but I really
don't like that. It does not belong there, XML document is pretty clean as of
yet, and it might still mean problems for XSLT etc. So I am thinking that
nsHTMLMapElement should be/contain nsImageMap. But there probably is a reason
why it is in the document rather than in the element. Anyway, I finally got some
traction on this, stomping along...

    
    

    
  
Removed the dependency to sink code refactoring, as this is not a sink issue.
No longer depends on: 21771

    
    

    
  
I attached two fixes 'cos I am not yet sure if the alternative fix 2 would work
always (it is cleaner, though, so I like it better).

The idea is to not store image map references in the HTML document, because we
can search a document (HTML, XML, anything...) for the map elements through the DOM.

In alt fix 1 the search is done by first searching for all the elements named
map, and then getting the first one of them that has a name that matches usemap.
This is closer to what the old code was doing, and will probably not break any
really bad HTML pages either. I developed this after alt fix 2 so there more
cleanup in this one.

In alt fix 2 the search is done simply by GetElementById(). This seems more
elegant, but if there is a broken page that has a non-map element with the same
id as the map element before the map, the image map would no longer work. I am
not 100% sure that we allow this, but my guess is yes. And I would also guess
that there are pages like that (dunno how much, though).

Opinions on which way to go?
Whiteboard: [fixinhand]

    
    

    
  
heikki: Personally I don't mind either way -- in valid pages there should be no 
difference in behaviour right?

While you're looking at this, could you consider making it easier to fix bug
1882 when we get around to it? That's the bug that is asking for "usemap" to 
work across documents, so for example you could store all your maps in a
"maps.xml" file and make all your documents on your website point to that file
for easy maintenance. If you use the DOM it should be relatively easy to put in
a hook for looking at a different document, right? Just a thought, don't worry
about actually implementing this or anything! :-)

    
    

    
  
Right, in valid pages there would be no difference (except the more elegant
solution would be slightly be faster).

Bug 1882 is outside the scope of this bug. But basic implementation shouldn't
even be difficult, maybe just making document.load() work for HTML as well so we
could load the other doc async etc.

    
    

    
  
If there's no difference for valid pages, then use the faster of the two 
patches. We're slow enough as it is, no need to add more slowdown! :-)

If we get enough bugs saying that we've broken pages that way, then we can
consider using your other patch...

How does that sound?

Regarding bug 1882: That's cool; I just wanted to make sure you didn't make it
harder to implement. Thanks! :-)

    
    

    
  
I have convinced myself that alt #1 is better: the usemap attribute points to an
element with name attribute (says HTML spec) so getElementById() should not even
work.

Also, I did some performance testing after waterson voiced concerns: loading
nsPresShell.cpp via LXR (1170kB file) and adding "#6900" in the URLbar and
pressing enter locates element A with attribute NAME whose value is "6900" near
the end of the document in the blink of an eye. To do that, we crawl the DOM
trying first getElementById() (which fails), then doing getElementsByName() and
walking the list searching for the first A element, doing case insensitive
string check, and finally scrolling. We do less work here, even though we have
to crawl the DOM as well. And typical image map documents contain a lot less HTML.

I am working on a cleanup, placing the common code in content/shared to be
shared between content and layout so that I do not need to duplicate code.

    
    

    
  
In the Proposed fix attachment I chose alt #1, and moved the duplicate finding
code into the brand new nsImageMapUtils which is shared between content and layout.

I believe this is functionally equivalent to the situation with the old
implementation, and as you can see by trying out the two evil testcases it will
work for unusual HTML. We gain performance in document destruction, reset and
map element's SetDocument() function. We loose performance in finding the image
map, but as I've said in earlier comments I believe this is so little you would
probably not notice. We gain smaller footprint because we can get rid of the
array in HTML document. We gain in more logical and easier to maintain code.

    
    

    
  
The makemapstresstest.pl script can generate testfiles so you can stress test
page loading time etc. By default it will generate a 222kB file that has a table
with 10 x 10 cells, each with an image and associated image map, each map with
45 area elements. In other words, the default test case has 100 images, 100
maps, and 4500 areas.

I am building an optimized version to profile with Quantify. I would also like
to run our page load tests but I do not yet know how, I've asked the owner of
that test to give a hand.

    
    

    
  
Uh oh, GetImageMap() is called in Paint(). If I am reading my Quantify data
right, I have slowed Paint() for about 15%. Even though I do not see the
difference on my machine, it could be significant on slower machines. The page
load measurement test harness is not much use here, but it would tell us exactly
how much the actual page load slowed down - that would probably be a lot less
than the 15% or so that I measured Paint() getting slower.

Anyway, I read the XHTML spec and it has officially deprecated the name
attribute, and tells to use id where name was previously used. HTML document has
a hash table of IDs, and I can do the same for XML.

I plan to leave the HTML side as it was, but when we have an XML document just
do getElementById() which will be fast with the hash table.

    
    

    
  
Allright, Proposed fix 2 is pretty uninvasive for plain ol' HTML. The duplicated
code is still in content/shared, where I first QI to HTML doc, and if failed, QI
to DOM doc and try getElementById(). Currently the FindImageMap method cannot
fail critically (like out of memory). To make us support maps in different
documents the FindImageMap method would be the place you would want to touch
first (see bug 1882).

I separated the getElementById() performance work for XML into its own bug: see
bug 75435. It shouldn't affect the landing of this because it does not affect
HTML, and image maps have not worked in XHTML at all before this so...

    
    

    
  
patch, id=30328, looks good. r=harishd

    
    

    
  
Recommended change: Use nsAReadableString instead of nsString in
nsImageMapUtils::FindImageMap(). Since you're doing a copy anyway, you can then
do the following:

nsAutoString usemap(nsPromiseFlatString(aUsemap));

You might even be able to avoid the copy and the cut if you do the following:

nsLiteralString usemap(((PRUnichar*)nsPromiseFlatString(aUsemap))+1,
aUsemap.Length()-1);

I'm sure there's a cleaner way to do it. scc or dbaron?

Other than that sr=vidur.

    
    

    
  
I changed to nsAReadableString (but can't use nsPromiseFlatString, it seems to
work without).

Fix checked in.

Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED

    
    

    
  
This is fixed in the April 18th build. Image maps are now working in a XHTML/XML
document. However, I did have a problem with the test case provided by the url.
The image was still acting like a single hot spot. After looking at the code, I
found out why. The image's usemap value and map's name value don't match.

<html:img src="images/02.jpg" usemap="#testMap1"/>
<html:map name="testmap1">

Changing the value in usemap to #testmap1 solves the problem.
Status: RESOLVED → VERIFIED

    
    

    
  
The examples are broken again, and the reason is that the samples have usemap
pointing to name attribute, while the code only searches for ID attribute (the
examples work if you replace name with id). I am not sure what to do: on one
hand the HTML 4.01 spec clearly says that is what should happen, but on the
other hand XHTML 1.0 says ID is the preferred method. I am inclined to leave it
as is...

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: