Bug 43293

Summary: HTMLStyleElement/SVGStyleElement need to share more code
Product: WebKit Reporter: Nikolas Zimmermann <zimmermann>
Component: WebCore Misc.Assignee: Nikolas Zimmermann <zimmermann>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, eric, gustavo, krit, webkit-ews, webkit.review.bot, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
none
Patch v2
none
Patch v3 krit: review+, krit: commit-queue-

Nikolas Zimmermann
Reported 2010-07-31 01:05:51 PDT
HTMLStyleElement and SVGStyleElement share the StyleElement base class. Some years ago the code was perfectly shared between those two classes. This has been changed since a while, and HTMLStyleElement contains functionality SVGStyleElement lacks (line number reporting for style sheets etc..) Refactor the code, to save virtual function calls, and to make both classes simpler.
Attachments
Patch (16.36 KB, patch)
2010-07-31 01:11 PDT, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
Patch v2 (16.60 KB, patch)
2010-07-31 01:22 PDT, Nikolas Zimmermann 		no flags
DetailsFormatted DiffDiff
Patch v3 (16.60 KB, patch)
2010-07-31 01:31 PDT, Nikolas Zimmermann 		krit: review+
krit: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Nikolas Zimmermann
Comment 1 2010-07-31 01:11:17 PDT
Created attachment 63141 [details] Patch
Early Warning System Bot
Comment 2 2010-07-31 01:19:37 PDT
Attachment 63141 [details] did not build on qt: Build output: http://queues.webkit.org/results/3646082
Eric Seidel (no email)
Comment 3 2010-07-31 01:22:05 PDT
Attachment 63141 [details] did not build on mac: Build output: http://queues.webkit.org/results/3648073
Nikolas Zimmermann
Comment 4 2010-07-31 01:22:49 PDT
Created attachment 63142 [details] Patch v2 Oops, a last minute change broke the build, restore m_sheet as protected variable.
Eric Seidel (no email)
Comment 5 2010-07-31 01:28:01 PDT
Attachment 63142 [details] did not build on mac: Build output: http://queues.webkit.org/results/3576716
Early Warning System Bot
Comment 6 2010-07-31 01:28:22 PDT
Attachment 63142 [details] did not build on qt: Build output: http://queues.webkit.org/results/3592647
Nikolas Zimmermann
Comment 7 2010-07-31 01:31:56 PDT
Created attachment 63143 [details] Patch v3 *sigh* uploaded wrong version.
WebKit Review Bot
Comment 8 2010-07-31 01:37:17 PDT
Attachment 63141 [details] did not build on chromium: Build output: http://queues.webkit.org/results/3635170
WebKit Review Bot
Comment 9 2010-07-31 01:40:05 PDT
Attachment 63141 [details] did not build on gtk: Build output: http://queues.webkit.org/results/3640128
WebKit Review Bot
Comment 10 2010-07-31 01:43:26 PDT
Attachment 63141 [details] did not build on win: Build output: http://queues.webkit.org/results/3559720
Dirk Schulze
Comment 11 2010-07-31 10:17:28 PDT
Comment on attachment 63143 [details] Patch v3 LGTM. Can you please add ASSERT's for document in StyleElement::insertedIntoDocument, removedFromDocument and sheetLoaded?
Nikolas Zimmermann
Comment 12 2010-07-31 10:31:25 PDT
Thanks, also added ASSERT(element) while I was at it. Landed in r64420.
Note You need to log in before you can comment on or make changes to this bug.