Closed Bug 216609 Opened 22 years ago Closed 21 years ago

Error - required <head> tag is missing

Categories

(SeaMonkey :: Composer, defect)

Product:
SeaMonkey
Component:
Composer
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: d_king, Assigned: d_king)

Details

(Keywords: regression, Whiteboard: Workaround: ensure your HTML uses lowercase "head" and "body" elements.)

Attachments

(1 file, 3 obsolete files)

regexp patch
2.09 KB, patch
glazou
: review+
Details | Diff | Splinter Review
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030718 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5a) Gecko/20030718 I was testing Bug #25141 by entering the HTML code from the example into Composer in "<HTML> Source" mode. When I select File-SaveAs I get the error "The HTML source could not be converted back into the document because the required <head> tag is missing. Please add it." The HTML I entered is as follows :- <HTML> <HEAD> <TITLE>Test</TITLE> </HEAD> </html> Note that <HEAD> and </HEAD> both exist. Reproducible: Always Steps to Reproduce: 1. From browser select File-New-ComposerPage 2. Enter HTML in "<HTML> Source" mode 3. Select File-SaveAs from same mode. Actual Results: Error message. Expected Results: Prompt for filename to save as. I've never accessed Composer this way. Up to now I've used Window-Composer. Selecting "Major" as can't use File-SaveAs, and error message is hopelessly wrong.
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5b) Gecko/20030728 Looks like the uppercase in <HEAD> or <BODY> is critical: 1.Paste this into a empty composer document (in "source" mode) - for me it doesn't seem to matter how I start Composer. <html> <head> <title>Test</title> </head> <body> </body> </html> 2. Changing <head> to <HEAD> or <body> to <BODY> will cause an error message while trying to save the file (Composer thinks one of the required tags are missing, ie <head> or <body>) Nothing happens if <html>, <title> or any end tag are written in uppercase letters - saving works fine then.
Changing Hardware and OS to ALL
OS: Windows XP → All
Hardware: PC → All
Yup, and I found the code that does it. http://lxr.mozilla.org/seamonkey/source/editor/ui/composer/content/editor.js#1848 is a function FinishHTMLSource() that includes the edit var beginHead = htmlSource.indexOf("<head"); but doesn't have an edit for the uppercase version, or maybe htmlSource is case sensitive when it shouldn't be (for this instance anyway).
The workaround is (of course) to always write the <head> and <body> tags in lowercase... So, adding a check for uppercase <HEAD> and <BODY> in FinishHTMLSource() will fix the problem? I wonder if there's a point in not allowing uppercases? CC:ing Harry Lu, who wrote the patch for bug 154521.
I'm currently testing some possible fixes for uppercase <BODY>. My first attempt using toLowerCase didn't work (probably due to my limited JS skills) so now I'm testing a brute force expansion of the If statement. As for bug #154521. It doesn't look like anyone considered uppercase as being an issue, mainly due to it being a quick fix for one particular problem. Hmmm, uppercase tags are legit aren't they? Oh well, I suppose I'd better check the spec to be sure.
Since I'm having a go at a patch for this bug, assigning to myself. From reading the W3 spec for HTML 4.01, I couldn't find any comment about what case <HEAD> should be, although all examples are in upper case. Also adding "regression" keyword as the fix for Bug #154521 caused this problem. That bug also fixed other problems, so I decided not to reopen it.
Assignee: composer → d_king
Keywords: regression
Attached patch Brute force patch (obsolete) — — Splinter Review
I'm not 100% happy with this patch, but it will fix the bug at hand. I'd rather use toLowerCase, but until I sort out how to use it, this is the solution.
Comment on attachment 130156 [details] [diff] [review] Brute force patch Requesting r, if anyone is listening at composer@editor.bugs
Attachment #130156 - Flags: review?(composer)
Attachment #130156 - Flags: superreview?(daniel)
Comment on attachment 130156 [details] [diff] [review] Brute force patch I am not super-reviewer. If you want me to r= this patch, please resubmit a request through the attachment manager, thanks.
Attachment #130156 - Flags: superreview?(daniel) → superreview-
Comment on attachment 130156 [details] [diff] [review] Brute force patch Do you know who the right person to do a sr is? Bugzilla just tells me that "composer@editor.bugs" is the owner!
Attachment #130156 - Flags: review?(composer) → review?(daniel)
Please download a more recent build and try again to hit the bug you describe here. I am using a WinXP 20030823 debug build and I can't reproduce.
Comment on attachment 130156 [details] [diff] [review] Brute force patch >- var beginHead = htmlSource.indexOf("<head"); >- if (beginHead == -1) >+ if (htmlSource.indexOf("<head") == -1 && htmlSource.indexOf("<HEAD") == -1 && htmlSource.indexOf("<Head") == -1) Urgh. Sorry, this is definitely too expensive and suboptimal. Please use a RegExp matching with case insensitivity. >- var beginBody = htmlSource.indexOf("<body"); >- if (beginBody == -1) >+ if (htmlSource.indexOf("<body") == -1 && htmlSource.indexOf("<BODY") == -1 && htmlSource.indexOf("<Body") == -1) Same thing here. See http://devedge.netscape.com/library/manuals/2000/javascript/1.3/reference/regex p.html#1193136
Attachment #130156 - Flags: review?(daniel) → review-
I've just duplicated the problem with the released Mozilla 1.5 beta (2003082704). As for the code being suboptimal, you are being too kind. In my mind, what I wrote was horrible, horrendous, and I should be first against the wall when the revolution comes (apologies to Douglas Adams). Many thanks for the link, I'll investigate that. There is an existing function to convert case, but I couldn't get it to work, hence the horrible thing I put up for review.
Attachment #130156 - Attachment is obsolete: true
Comment on attachment 130712 [details] [diff] [review] Convert to lowercase before compare. A much better patch. Convert the source to lowercase before doing the compare. I used "toLowerCase" as it's used all over the place in editor.js.
Attachment #130712 - Flags: review?(daniel)
Attachment #130712 - Flags: review?(daniel)
Attached patch Even better patch (obsolete) — — Splinter Review
Don't convert zero-length strings to lower case.
Attachment #130712 - Attachment is obsolete: true
Comment on attachment 130713 [details] [diff] [review] Even better patch This one should be better. Seeking "r".
Attachment #130713 - Flags: review?(daniel)
Attachment #130713 - Flags: superreview?(sfraser)
Attachment #130713 - Flags: review?(kaie)
Attachment #130713 - Flags: review?(daniel)
Comment on attachment 130713 [details] [diff] [review] Even better patch > var htmlSource = gSourceTextEditor.outputToString(kTextMimeType, 1024); // OutputLFLineBreak > if (htmlSource.length > 0) > { >+ htmlSource = htmlSource.toLowerCase(); > var beginHead = htmlSource.indexOf("<head"); > if (beginHead == -1) > { This is still suboptimal. You're converting the entire source (which may be several MB of text) to lowercase just to do compares. Please follow up on Daniel Glazman's suggestions above of using a regex.
Attachment #130713 - Flags: superreview?(sfraser) → superreview-
Hmmm, that's a point, htmlSource could be rather large. New patch coming....
Comment on attachment 130713 [details] [diff] [review] Even better patch Cancelling review request.
Attachment #130713 - Attachment is obsolete: true
Attachment #130713 - Flags: review?(kaie)
Added info to whiteboard for those creating HTML 4.x or below web pages which accept uppercase elements (unlike XHTML). I'm still fighting with regexp. I have the code, but it's being difficult about working properly.
Status: NEW → ASSIGNED
Whiteboard: Workaround: ensure your HTML uses lowercase "head" and "body" elements.
Attached patch regexp patch — — Splinter Review
OK, after using Venkman to confirm my pattern matching, and the correct method of using the "i" flag, I have this patch. Tested on Mozilla with <HEAD>, <Head> and <head>. Also, I found another copy of editor.js that hides in mozilla/composer/base/content which appears to be an exact duplicate of the one in mozilla/editor/ui/composer/content. Both copies are in this patch.
Comment on attachment 137488 [details] [diff] [review] regexp patch Asking for r and sr.
Attachment #137488 - Flags: superreview?(sfraser)
Attachment #137488 - Flags: review?(kaie)
Please ask another editor person for review. I think I should no longer be a peer of editor, I don't know enough about it to do reviews.
Comment on attachment 137488 [details] [diff] [review] regexp patch As per request, changing reviewer.
Attachment #137488 - Flags: review?(kaie) → review?(neil.parkwaycc.co.uk)
glazou: you need to use File/Save As to trigger the bug, clicking on Normal mode will simply ignore any document without <html>, <head> or <body> tags.
Comment on attachment 137488 [details] [diff] [review] regexp patch Changing reviewer.
Attachment #137488 - Flags: review?(neil.parkwaycc.co.uk) → review?(cmanske)
Comment on attachment 137488 [details] [diff] [review] regexp patch .
Attachment #137488 - Flags: review?(cmanske) → review?(daniel)
Product: Browser → Seamonkey
Attachment #137488 - Flags: review?(daniel) → review?(composer)
Simple fix, just needs minimal attention to review and check-in.
Flags: blocking1.8b?
Flags: blocking-aviary1.1?
Too late for 1.8b1, moving nomination to 1.8b2, but the real trick is to get reviews on the patch and get it in.
Flags: blocking1.8b?
Flags: blocking1.8b2?
Flags: blocking1.8b-
Attachment #137488 - Flags: review?(composer) → review?(daniel)
Attachment #137488 - Flags: superreview?(sfraser_bugs) → superreview?(composer)
This just needs a "sr" and it's ready to be checked in. Very very minor fix.
Flags: blocking-seamonkey1.0a?
Comment on attachment 137488 [details] [diff] [review] regexp patch you should request sr of a specific person...
I just used the information at https://bugzilla.mozilla.org/describecomponents.cgi?product=Mozilla%20Application%20Suite which I got to by following the "Component" link.
Attachment #137488 - Flags: superreview?(composer) → superreview?(daniel)
If we were able to ship stable releases with this, we won't hold an alpha of the new suite for that. Please request sr= from a real super reviewer though (there should be a list of them on mozilla.org - a tip: look who did sr on patches checked in in the last couple of days/weeks)
Flags: blocking-seamonkey1.0a? → blocking-seamonkey1.0a-
(In reply to comment #34) > I just used the information at > https://bugzilla.mozilla.org/describecomponents.cgi?product=Mozilla%20Application%20Suite > which I got to by following the "Component" link. That list is unrelated to reviewers and super-reviewers. see http://mozilla.org/hacking/reviewers.html for the list of super-reviewers.
Attachment #137488 - Flags: superreview?(daniel) → superreview?(neil.parkwaycc.co.uk)
(In reply to comment #35) > If we were able to ship stable releases with this, we won't hold an alpha of the > new suite for that. I'm not sure why Asa set the blocking-seamonkey1.0a to ?.
(In reply to comment #36) > That list is unrelated to reviewers and super-reviewers. see > http://mozilla.org/hacking/reviewers.html for the list of super-reviewers. According to that link, Neil is the only super-reviewer for composer. Changed the flag to ask him for a sr.
Since attachment 150948 [details] [diff] [review] and its follow-up fix were checked in last July, most of FinishHTMLSource should no longer be necessary.
Hmm... although I filed bug 252423 I forgot to follow up on it :-[
(In reply to comment #37) > I'm not sure why Asa set the blocking-seamonkey1.0a to ?. he moved blocking-1.8b2? requests to blocking-seamonkey1.0a? in the moz. app suite product.
OK, so part of the fix for Bug #192557 may have fixed this problem as well. Working for over a year to get a r and sr only to find that it was probably fixed elsewhere..... Oh well, I suppose I should double check the fix.
Fixed by part of checkin to Bug #192557. Bug #252423 will remove the code that this bug patches. Fixed.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment on attachment 137488 [details] [diff] [review] regexp patch Removing obsolete request.
Attachment #137488 - Flags: superreview?(neil.parkwaycc.co.uk)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: