Bug 43293 - HTMLStyleElement/SVGStyleElement need to share more code
Summary: HTMLStyleElement/SVGStyleElement need to share more code
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nikolas Zimmermann
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-07-31 01:05 PDT by Nikolas Zimmermann
Modified: 2010-07-31 10:31 PDT (History)
7 users (show)

See Also:


Attachments
Patch (16.36 KB, patch)
2010-07-31 01:11 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v2 (16.60 KB, patch)
2010-07-31 01:22 PDT, Nikolas Zimmermann
no flags Details | Formatted Diff | Diff
Patch v3 (16.60 KB, patch)
2010-07-31 01:31 PDT, Nikolas Zimmermann
krit: review+
krit: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolas Zimmermann 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.
Comment 1 Nikolas Zimmermann 2010-07-31 01:11:17 PDT
Created attachment 63141 [details]
Patch
Comment 2 Early Warning System Bot 2010-07-31 01:19:37 PDT
Attachment 63141 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3646082
Comment 3 Eric Seidel (no email) 2010-07-31 01:22:05 PDT
Attachment 63141 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3648073
Comment 4 Nikolas Zimmermann 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.
Comment 5 Eric Seidel (no email) 2010-07-31 01:28:01 PDT
Attachment 63142 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3576716
Comment 6 Early Warning System Bot 2010-07-31 01:28:22 PDT
Attachment 63142 [details] did not build on qt:
Build output: http://queues.webkit.org/results/3592647
Comment 7 Nikolas Zimmermann 2010-07-31 01:31:56 PDT
Created attachment 63143 [details]
Patch v3

*sigh* uploaded wrong version.
Comment 8 WebKit Review Bot 2010-07-31 01:37:17 PDT
Attachment 63141 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/3635170
Comment 9 WebKit Review Bot 2010-07-31 01:40:05 PDT
Attachment 63141 [details] did not build on gtk:
Build output: http://queues.webkit.org/results/3640128
Comment 10 WebKit Review Bot 2010-07-31 01:43:26 PDT
Attachment 63141 [details] did not build on win:
Build output: http://queues.webkit.org/results/3559720
Comment 11 Dirk Schulze 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?
Comment 12 Nikolas Zimmermann 2010-07-31 10:31:25 PDT
Thanks, also added ASSERT(element) while I was at it. Landed in r64420.