Spot the Error 3: Calling all Sleuths!

Code Sleuth Heaven!
[rule=“100%”]Orange[/rule]
Here is the third Spot the Error Competition. (If you missed the first two, you can find them here and [URL=“http://www.sitepoint.com/forums/showthread.php?870609-quot-Spot-the-Error-quot-Competition-2-Olympic-Edition”]here.)

The quoted text below (a mock forum question) has quite a few coding errors in it. All you have to do is spot as many errors as you can and list them in your reply. This mock post was cunningly crafted by our own xhtmlcoder, who has a knack for posing curly conundrums and tricky traps for the unsuspecting! Some of the errors below are trickier than others (and some are just plain mean!) so there’s a lot of scope for you to be a real sleuth and show off your coding expertise.

There is extra credit for explaining why something is an error. The winner will be the person who spots the most errors, or who gives the best explanation of what the errors are. (So don’t be discouraged if someone beats you to it. Just give a better explanation.)

xhtmlcoder himself will be the judge of this competition, and the prize for the winner will a free SitePoint ebook.

Remember that this is just for fun, and lively debate is welcome! Good luck!

Edit:

K guys, as of March 12th, xhtmlcoder is going to assess the entries so far to pick a winner, but feel free to keep the thread going. :slight_smile:

[rule=“100%”]Orange[/rule]
So, here is the pretend post. How many errors can you spot?

Interesting markup. I have as many comments as flat-out errors.


<!-- Copyright (C) 2012, Frobozz Training. All Rights Reserved. -->
<?xml version="1.0" encoding="UTF-8"?>
<?xml-stylesheet href="frobozz.css" type="text/css"?>

Crap before doctype (though that only matters to IE6 and 7 nowadays, and neither of them can really read this markup)
The XML declaration isn’t bad but I doubt that’s what the newbie was going for
The second XML declaration actually is also ok but also doubt that’s what the newbie was going for.

The OP may have totally meant to use Basic, but then in that case I guess we’d recommend they skip it if they want to target mobiles cause nobody has a Gordon Gecko brickphone anymore.


<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML Basic 1.0//EN"
    "http://www.w3.org/TR/xhtml-basic/xhtml-basic10.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en">

Technically the only thing wrong here is Basic (for mobiles) has a more recent version, 1.1.


  <head>
    <[color=red]T[/color]itle>

But if one is going to use XML, lowercase matters. Real Error #1.


      Frobozz Training: Search Course Database
    </title>
    <link rel="stylesheet" href="frobozz.css" type="text/css" />

I suppose the re-stating of the stylesheet in the head will simply override the one listed earlier.


    <[b]style[/b] type="text/css">
       body {  
         background-color: grey;
         }
         /* This section commented out for testing 
        div { margin:100px } /* main section*/
      p {
         margin: 0 1em
        }
      finished testing
        */
    [color=red]</script>[/color]<style type="text/css">
     th {
      color: silver;
     }  
    </style>

Basic 1.1 supports style tags. I’m not certain but I didn’t think 1.0 did.
Basic 1.1 supports script tags, but 1.0 did not. Error closing script tag without opening script tag anyway. Real error #2.

Without the error closing script tag, the browsers running this page as XHTML Basic 1.0 may have simply ignored the style tags in the head and moved on. Instead there will now be a parsing error and the world will stop turning.


  </head>
  <body>
    [color=red]<img src="Logo" title="This a Great Logo" alt="This a Great Logo">[/color]

inlines may not be a direct child of the body element. Img is EMPTY and must be closed />. src must be a real URI though depending on the backend this might be a valid URI. 2 real errors.


    <div id="box1">
      <div id="_content">
        [b]<h2>[/b]
          Course Search Page
        </h2>

I would not start with an h2. Leading underscore should be totally valid for id name though.


        <p>
          Below is a Course Search form enabling you the user to
          search our database on see how many students have
          enrolled on any of our courses which can be selected
          via the dropdown menu:
        [color=red]</P>[/color]

capitals no no in XML. 1 real error.


        <form action="search.php" method="get">
          <div>
            <label>Search Places Available by Subject: <select>
              <option>
                Basic IT
              </option>
              <option>
                Web Page Design L2
              </option>
              <option>
                Web Page Design L3
              </option>
            </select></label> <input type="submit"
            values="Search..." />
          </div>
        </form>

I think the above is technically correct, since the OP only has Basic forms available since this is Basic version 1.0. I would argue different semantics if the doctype were otherwise, like fieldsets and not wrapping the select with a label, etc.


        <h2>
          Table of Search Results
        </h2>
       <table>
         <tr>
          [color=red]<td>
           <th>Course Name</th>[/color]

The td must be closed before opening a th. Td’s cannot be direct parents of th’s. Since this is XML we don’t get any implied closing block elements. 1 real error.


            <table [color=red]border="1"[/color] summary="Results Table">
            [color=red]<tr>
            <h2>
              Course Code
            </h2>[/color]
            <th><b>
              <acronym title="Maximum">Max</abbr> <abbr title="Number">no.</abbr></b>
            </th>

Table cannot be a direct child of tr.
Basic 1.0 tables can’t have border attributes. Tr’s must have th or td as direct children.
Abbr and acronym and b are all fine though. If this were a different doctype, I would recommend different semantics.
Would recommend h2 become an h3. 3 real errors.


            <th>
              <abbr title="Number">No.</abbr> enrolled
            </th>
          </tr>
          <tr>
            <td>
              Basic IT
            </td>
            <td>
              BIT01
            </td>
            <td>
              30
            </td>
            <td>
              18
            </td>
          </tr>
          <tr>
            <td>
              Web Page Design L2
            </td>
            <td>
              WEB02
            </td>
            <td>
              20
            </td>
            <td>
              15
            </td>
          </tr>
          <tr>
            <td>
              Web Page Design L3
            </td>
            <td>
              WEB03
            </td>
            <td>
              16
            </td>
            <td>
              9
           </tr>
           </table>
          [color=red]</td>[/color]
         </tr>
        </table>

Here’s the closing tag. If there hadn’t been a direct-child th earlier, this closing td wouldn’t have been considered an error.


        <p>
          Lorem ipsum dolor sit amet, consectetuer adipiscing
          elit. Mauris sodales, lacus quis vestibulum faucibus,
          velit purus lacinia erat, eu suscipit felis urna ut
          sapien.
        [color=red]<p>
        </p>
          Lorem ipsum dolor sit amet, consectetuer adipiscing
          elit. Mauris sodales, lacus quis vestibulum faucibus,
          velit purus lacinia erat, eu suscipit felis urna ut
          sapien.
        </p>[/color]

Code not well-formed: tag nesting mismatch.


        <div class="footer">
          <p>
            [<a href=[color=red]".../home.htm"[/color]
            title="Return to Home Page">Return Home</a>]
          </p>
          <p>
          <b>
            Copyright &#copy; 2012, Frobozz Training. All Rights
            Reserved.
          </b>
          </p>
          [color=red]<div />[/color]

href doesn’t appear to be a valid URI.
I think if this were purely considered XML the div would be fine. Since this is XML as Basic XHTML this may not be allowed. I would run it through the Basic validator to know for sure.


        </div>
      </div>
    </div>
  </body>
</html>

My take on the first one.

Good attempt, though there are still quite a few you haven’t caught yet… Chocolate mouse catcher. At least one, I do know Paul would see. But you do get “Extra Bonus points” for being the first person to dare make a ‘first-attempt’. So at least I know you haven’t copied off anyone. So keep trying; so far you’ve spotted some, have another look and you’ll find some more to add to your list…

Actually, Poes I think most of the SPF members are too sacred to enter - I promise not to bite. Else they’re waiting for the first intrepid explorer to give them a clue where “X” marks the spot. :slight_smile:

Paul would find [spoiler] the mismatching CSS comments /* */ in


 <style type="text/css">
       body {  
         background-color: grey;
         }
         /* This section commented out for testing 
        div { margin:100px } /* main section*/
      p {
         margin: 0 1em
        }
      finished testing
        */

I also initially missed the mis-nested <style> tags in the <head>… but now it explains the closing script tag was probably meant to be style.
[/spoiler]

It’s been ages since I’ve had the time to take something like this on. I’m sure I missed quite a few (and I’m sure once I read Poes, I’ll be slapping my head).

Search Page Code

  1. Not really an error, but an fyi - <?xml directives will cause IE to go into quirks mode.
  2. Is XHTML basic really what the user wants? It’s supposed to be limited to mobile only. Strict or Transitional are typically more appropriate.
  3. html element should have a lang=“en” attribute
  4. Title element has capitalization difference - xhtml elements must be lower case (see: www.w3.org/TR/xhtml1/)
  5. / on / main section */ will cause css rendering issues
  6. No need for second style element. Should combine with first.
  7. img is a self closing element so should end in />. Src is also bogus
  8. p element capitalization error. Must be lower case - see: www.w3.org/TR/xhtml1/
  9. headings aren’t appropriate - first heading should be h1, next section h2, and such. Size should be controlled via css.
  10. no need for div inside of form
  11. label has no for attribute to tie it to element (not critical, but helpful)
  12. no name attribute on select
  13. No values for option elements
  14. values is not a valid attribute - should be value
  15. Search Results Table is toasted.
    [LIST=1]
  16. Course Name th is “nested” inside a td. Move it above the td and wrap it in a tr element.
  17. Results Table is supposed to be a nested table, but is not included in any element.
  18. h2 is a block element, which cannot be in another block element. It either needs to be a th
  19. the first row of that table is missing an element (column heading)
  20. on base behavior, there’s no need for the bold inside the th, which displays as bold natively.
    [/LIST]
  21. the closing paragraph tag is swapped with the opening paragraph tag for the next paragraph.
  22. href in footer has an incorrect relative reference the … should be …
  23. empty div in the footer - no point…

Feedback Page Code

  1. Not really an error, but an fyi - <?xml directives will cause IE to go into quirks mode.
  2. type=“application/javascript” is not valid for the xml-stylesheet. Should be type=“text/css”
  3. Is XHTML basic really what the user wants? It’s supposed to be limited to mobile only. Strict or Transitional are typically more appropriate.
  4. script element has no src, nor any inside text
  5. div id elements should NOT start with numbers
  6. comment should be not in the middle of the strong element
  7. headings are not appropriate. h2 should be h1, h6 should be h2. Sizes should be styled via css
  8. mailto:example@@example.com should be mailto:example@example.com
  9. input elements does not support the alt attribute except for images
  10. placeholder is an html5 attribute, not xhtml
  11. rows should say 10, not 1o
  12. href should use title instead of alt

Actually…

[spoiler]
dave said:
no need for div inside of form

However forms may not contain inlines as direct children… so some block is needed, and can’t be a fieldset since Basic1.0 doesn’t have those.

dave said:
label has no for attribute to tie it to element (not critical, but helpful)

However since the inputs are inside the labels, they are implicitly linked (with for-id pairs they are explicitly linked… it is not recommended to do both at the same time though you could). Meaning other than IE6 (who can’t read this markup anyway), you should get the same behaviour as a for-id setup would.

dave said:
No values for option elements

It’s an implied attribute if not explicitly listed. I suppose the idea was the form should send the implicit values (which would be the cdata between the option tags) instead.[/spoiler]

Dave definitely caught a few I missed.

Good job I can see more than both of you. :wink: Though yes apart from one a two minor mistakes Dave, some which Poes mentioned we are getting a little closer.

Ralph, I wonder if your SPF Design Team dare show their faces and enter? LOL >;-)

I’m not super familiar with Basic so I’m not surprised I missed those - like I said, I thought it might be a bad docType choice which is why I answered the way I did.

As for the rest, now you made me go look…there’s a couple bonheaded ones I missed for sure… sigh

Search Page Code

  1. Display issue - assuming there’s no background-color defined in the css file, color: silver on the th will be almost invisible on the background color: grey of the page.
  2. The copyright html entity is wrong (shoulda caught that the first time - grrrr)
  3. technically, the <b> should be <strong>, but that’s one of those contested deprecations which people still use, so I let it go the first time…
  4. I kinda covered it when I mentioned that the second style tag wasn’t needed (meaning that middle should be removed, but there’s a closing script element which should be style.
  5. There’s an acronym opening element, but an abbr to close.

Feedback Page Code

  1. Design type - There are a lot of <br /> elements, which while technically can be used like this, would be better served handled via css (especially the one after the heading)
  2. the enc type on the form is wrong it should be text/plain not text\plain - I had actually caught that the last time but forgot to denote it. oops!
  3. semantically, the <br /> elements should be outside the label elements (though see #1 regarding the use of them…)

Yes, I purposely choose the code to make it tougher as that member of the Family has elegance - without me giving away any spoilers. I knew where you were coming from so it was all good. Discussion is good. Explanations are also good, they show your thought processes, and help exercise the grey-matter etc. :slight_smile:

Yeah I thought the XHTML Basic doctype was interesting enough I decided to go on as if the OP were going to stick to that doctype, for whatever reason… makes it more interesting. Especially since Basic 1.0 is so much more primitive than Basic 1.1, it leaves out so many attributes and elements we take for granted with full XHTML/HTML.

If someone were to say “well, the OP should change the doctype to X” then it would be a whole different set of errors and recommendations which makes this even more interesting.


And Dave…
Design type - There are a lot of <br /> elements
Also, the prominent space in there that was a workaround for Nutscrape 4, or 7, I forget which… since for everyone else in the universe <br/> would be just fine : P

Dave caught even more that I also should have seen… esp the copy and mismatched acronym/abbrs and valueS attribute…

Haha, I did expect Eric Watson or Dresden Phoenix to be in here already :stuck_out_tongue: But it’s early.

My contribution to the Search Page Code (with apologies for repeating anything already identified).
I included some surrounding lines so the location might be clear.


                <h2>    <!-- SHOULD NOT BE INSIDE TABLE -->
                    Course Code
                </h2>
                <table border="1" summary="Results Table">
                <tr>  <!-- ONLY 2 TH TAGS but 4 COLUMNS and these seem to be for columns 3 and 4 -->
                    <th><b><abbr title="Maximum">Max</abbr> <abbr title="Number">no.</abbr></b></th>
                    <th><abbr title="Number">No.</abbr> enrolled</th>
                </tr>

The number of <th> tags does not match the number of columns in the succeeding rows (should fit 4 columns). As is, they would reside over the wrong columns.


                    <td>
                        16
                    </td>
                    <td>
                        9
                    </td>  <!-- MISSING TAG -->
                </tr>
                </table>
            </td>

We seem to be missing a close </td> tag in the last row of this table


          <b>
            Copyright &#copy; 2012, Frobozz Training. All Rights
            Reserved.
          </b>
          </p>
<!--      <div />    <!-- SHOULD BE DELETED -->
        </div>
    </div>
</div>
</body>

The ill-formed <div /> tag should be deleted, lest there be too may </div>s.

The only things I noticed on the Feedback page that I don’t think have been mentioned are:


<script language="JavaScript" type="text/javascript" />

Line 11: the <script> tag is improperly closed </script>


offered by <strong <!-- class="important"-->>Frobozz

Line 21: malformed <strong> tag; the gt symbol is out of place.

So far we’ve had a couple of Code cats try; two SPF staff members and one past, so far they are on the right path. Though there are still some more errors I can see. So keep trying guys. :wink:

I wonder if the low entry number so far, is because the rumours are true; many of the members on SPF require WYSIWYG Editors to do their coding? :wink:

It would certainly explain why only 3 thus far have had the courage to enter. I commend those three so far for spotting several errors and each one by the looks of things have seen different ‘errors’. Usually the first three are the smartest because they don’t need to copy but surely someone else can find/fix some more errors…

Hodwy,

I’m not sure how much of a sleuth I am, but this is what I noticed:

Search Page code:[spoiler]

[LIST=1]
[]No Character encoding was declared within the <head> section of the document.
[
]The <title> tag starts with a capital “T”
[]There is a closing </script> tag which was never opened
[
]Following that there is a closing </style> tag missing
[]The <img> tag should be shut with “/>” instead of “>”
[
]Paragraph shuts with an uppercase </P>. This is an error
[]In this “values” should be singular: <input type=“submit” values=“Search…” />
[
]The table is malformed, having more cells than headings
[]<acronym title=“Maximum”>Max</abbr> mismatching tags
[
]<div /> is obviously wrong
[*]&#copy; should have been ©
[/LIST][/spoiler]Feedback Page code:[spoiler]

[LIST=1]
[]This just looks wrong: xml:lang=“en” lang=“en”
[
]language=“Javascript” is wrong: <script language=“JavaScript” type=“text/javascript” />
[]The id cannot start with a digit: <div id=“1box”>
[
]There is a malformed <strong> tag: <strong
[]Placeholder attribute wasn’t introduced until HTML 5: placeholder=“First name”
[
]The input cannot have an alt attribute: <input name=“name” … alt=“Please enter your …
[*]The anchor tag cannot have an alt attribute: <a href=“home page.htm” alt=”
[/LIST][/spoiler]It must be said that the mark-up is really tricky and several times I thought I’d found an error, when I hadn’t.
Hat’s off, Robert!

Good attempt Dave. Yes, you spotted some good tricky ones there and it’s nice to see the SPF Programming Team have now put forward a competitor. Though to everyone there are still some more errors so keep 'em coming, we are getting closer. Come on give it a try even if you are a newbie you should spot some errors don’t feel embarrassed if you cannot find many we’ve had four people attempting and they found different errors and all missed a few. :slight_smile:

Haven’t had much time to look at this but here’s a couple of comments on the css.


<link rel="stylesheet" href="frobozz.css" type="text/css" />
<style type="text/css">
body { background-color: grey; }
/* This section commented out for testing 
        div { margin:100px } /* main section*/
      p { margin: 0 1em }
 finished testing  */  </script><style type="text/css">  th {
 color: silver;
}
</style>

(Assuming the basic doctype was a mistake otherwise the style tag and type attribute would not be valid for the document.)

  1. There’s no media attribute on the link tag which is not invalid but should be specified for the required media.

  2. body { background-color: grey; }

There’s no colour keyword for “grey” in css2.1. It is “gray” to be valid. Grey has been added to css3 in the extended colour keyword module. IE6 will ignore grey and uses only “gray”. It would also be more concise to use the shorthand “background” instead of “background-color” as no other rules have been applied.

  1. The section has been commented out but the author has not noticed that there were nested comments in the middle of the section which will have the effect of cutting the comments short.

Therefore the only section commented out will be this:


/* This section commented out for testing 
        div { margin:100px } /* main section*/

Which will leave the only valid (css2.1) code to be this:


  p { margin: 0 1em }

  1. Setting a 1 em side padding on all p elements is a silly thing to do globally as it is unlikely you would want all p elements styled like that. The same applies to div{margin:100px} if it wasn’t commented out. Specifying styles globally like that lead to maintenance problems where you have to continually over-write settings you have made. A default rule would be one that you are likely to use in most cases.

  2. Due to the nested comments the following section will be invalid:


 finished testing  */  </script><style type="text/css">  th {
 color: silver;
}

  1. The closing script tag is an error and should be a closing style tag and although the new style element is ok the rules could have been included in the first one to save code.

  2. If the nested comments were fixed then the contrast between the grey background colour and the silver of the th would be no good and the text be almost invisible.

I probably missed the obvious ones as its getting late here and I’m tired.:slight_smile:

Paul, it looks like you spotted most of those CSS issues I wrote. Even after a full day’s work you still produce the goods… I think how you phrased the ‘potential CSS issues’ might be slightly better that I would have explain them, when I provide the Judge’s Feedback next week sometime - all good catches and well explained. :slight_smile:

Regarding everyone who has yet to compete and any of the others that want to find the other “unspotted” errors keep trying there are still a few left to detect.

Only other thing I see (other than the blatantly obvious stuff Paul caught that I should have…) short of spelling/grammar errors is

css files are different from one page to the other. Not necessarily wrong, but may cause consistency problems.

Well, spotted, yes that is the one I was thinking of regarding CSS. Like you say more a discussion point, which could lead to unexpected results - as far as the web author/browser goes - there was a reason for two web page files to compare and contrast with both. I think that more-or-less covers all the CSS pitfalls I wrote.

There is at least another - very subtle and sneaky issue to find almost Red herring - that is a similar vein - though not CSS.

Actually Ralph @ralph_m ; might spot that one, he likes wordplay and the clue is clearly visible in the last sentence - in my prior post - and elsewhere other than post #1 and it isn’t Greek.