Bug 12646 - REGRESSION: RapidWeaver 's "Hello" HTML page doesn't display any DHTML effects when clicking on it's links
Summary: REGRESSION: RapidWeaver 's "Hello" HTML page doesn't display any DHTML effect...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P1 Normal
Assignee: Geoffrey Garen
URL:
Keywords: HasReduction, InRadar
Depends on:
Blocks:
 
Reported: 2007-02-06 23:19 PST by Maciej Stachowiak
Modified: 2007-06-21 16:06 PDT (History)
2 users (show)

See Also:


Attachments
Reduction (242 bytes, text/html)
2007-02-19 17:14 PST, Jeff McGlynn
no flags Details
nested heading tags test & results (7.56 KB, application/octet-stream)
2007-04-13 11:09 PDT, Geoffrey Garen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Maciej Stachowiak 2007-02-06 23:19:08 PST
2007-02-05 15:58:39 Chris Petersen:
* SUMMARY
Under TOT Webkit, RapidWeaver 's "Hello"  HTML page doesn't display any DHTML effects when clicking on it's links. This does work under RapidWeaver under stock 10.4.8 webkit frameworks.

* STEPS TO REPRODUCE
1. Download and install http://www.realmacsoftware.com/rapidweaver/
2. Launch RapidWeaver 3.5.1 under stock 10.4.8
3. After main window opens in RapidWeaver , you should see a page that states "Say Hello to RapidWeaver"
4. Click on "Get Started" link and notice the DHTML transition that occurs on page
5. Quit and relaunch RapidWeaver under TOT webkit
6. Repeat Step 4. Notice the entire page content is displayed and clicking on links such as ""Get Started" are no longer functional.

* RESULTS
The HTML page should be displayed correctly and be functional but isn't.

* REGRESSION
Yes, this works in RapidWeaver under stock 10.4.8

2007-02-05 16:14:28 Chris Petersen:
You can reproduce this issue under TOT (Tiger) and TOT (Leopard) with the following steps:

*Enable Debug menu before completing these steps

1) Download the attached file "RapidWeaver.zip" and uncompress it.
2) Open placeholder_eng.html in TOT 
3) Notice the problem as originally described

Terminal shows this output:

[627] file:///Users/cp/Desktop/RapidWeaver/placeholder_eng.html line 19: TypeError: Value undefined (result of expression fx.MultiFadeSize) is not an object. Cannot be used with new.

While JS console displays this error:

TypeError: Undefined value
file:///Users/cp/Desktop/scripts/moo.fx.pack.js      Line 121

Looking at the JS file in BBEdit shows , this code on that line :

hide: function(el, mode){
	el.fs.hide(mode);
 	}

2007-02-05 16:18:21 Chris Petersen:
This problem also happens in stock Leopard9A321 as well.

<rdar://problem/4977124>
Comment 1 Alexey Proskuryakov 2007-02-08 09:06:46 PST
(In reply to comment #0)
> 1) Download the attached file "RapidWeaver.zip" and uncompress it.

I t would be nice to have it in Bugzilla, if possible.
Comment 2 Jeff McGlynn 2007-02-19 17:14:42 PST
Created attachment 13259 [details]
Reduction

This document produces an alert containing "header" in WebKit R19672 or "content" in Safari 419.3, which causes an unexpected node to be retrieved when using nextSibling in Webkit for RapidWeaver's "Hello" page.
Comment 3 Alexey Proskuryakov 2007-02-24 09:08:32 PST
Looks like the regression was introduced in r9639 <http://trac.webkit.org/projects/webkit/changeset/9639>, as HTMLParser::isInline() no longer pays attention to the actual element style, and only uses a hardcoded list of tag names.

Despite this change having been hidden in a huge unrelated patch, the new behavior appears to match Firefox better, as it also doesn't seem to pay attention to the actual style - but its parser allows nested headers with only inline elements between them (for some inline elements).
Comment 4 Geoffrey Garen 2007-04-12 13:05:34 PDT
Here's what the DOM looks like for the relevant snippet.

Safari 2.0 DOM:
BODY
|-->H3
    |-->A
        |-->H4
            |-->#text
|-->DIV
    |-->#text

Firefox DOM:
BODY
|-->H3
    |-->A
        |-->H4
            |-->#text
|-->DIV
    |-->#text

IE 6 DOM:
BODY
|-->H3
    |-->A
        |-->H4
            |-->#text
|-->DIV
    |-->#text

TOT DOM:
BODY
|-->H3
    |-->A
|-->A
    |-->H4
        |-->#text
|-->DIV
    |-->#text
Comment 5 Geoffrey Garen 2007-04-12 19:52:14 PDT
Another change you could roll out to fix this is http://trac.webkit.org/projects/webkit/changeset/7701. It looks like the rule added there was inappropriately general. Unfortunately, the check-in lacked a ChangeLog or bug number, so we don't know exactly what it was trying to fix. We'll have to do some aggressive testing to avoid causing a regression.
Comment 6 Geoffrey Garen 2007-04-12 21:15:02 PDT
I was confused by trac. Here's the ChangeLog entry:

"Don't allow nested headers when only inlines are in between them.  Fixes a hang related to pathological nesting on magicmethodsonline.com."

The page in questino was http://magicmethodsonline.com/latest.html
Comment 7 Geoffrey Garen 2007-04-12 22:26:16 PDT
I found this comment in bug 3405 to explain using the list of tag names:

"This patch implements the isInline method in the parser and stops using renderers to make the determination (thus eliminating any potential variance in rendering caused by deferred renderer construction)."
Comment 8 Geoffrey Garen 2007-04-12 22:34:36 PDT
bug 13337 is a very similar hang due to deep nesting.
Comment 9 Geoffrey Garen 2007-04-13 11:09:21 PDT
Created attachment 14026 [details]
nested heading tags test & results

Attached a test for nested heading tags along with results from different browsers. Behavior seems pretty randomly distributed.
Comment 10 Geoffrey Garen 2007-04-14 19:02:02 PDT
I can't reproduce the hang at magicmethodsonline.com, even if I roll out r7701.
Comment 11 Geoffrey Garen 2007-04-16 13:54:54 PDT
Dave mentioned that the magicmethodsonline.com depends on the content of the day, so a larger page on another day would probably still show the hang.
Comment 12 Ian 'Hixie' Hickson 2007-06-19 03:14:05 PDT
I've changed the spec for compatibility with IE7 -- now, headers nest (regardless of other elements involved). This should get you compatibility for this bug, but it's not clear if it'll break other things or not. Please let me know if this is a problem or not.

   http://html5.org/tools/web-apps-tracker?from=925&to=926
Comment 13 Geoffrey Garen 2007-06-21 16:06:45 PDT
Committed revision 23728.