Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(698)

Issue 1322543004: CSSStyleSheet::insertRule should respect the default namespace (Closed)

Created:
5 years, 3 months ago by Timothy Loh
Modified:
5 years, 3 months ago
CC:
darktears, apavlov+blink_chromium.org, blink-reviews, blink-reviews-css, blink-reviews-style_chromium.org, dglazkov+blink, rwlbuis
Base URL:
https://chromium.googlesource.com/chromium/blink.git@master
Target Ref:
refs/heads/master
Project:
blink
Visibility:
Public.

Description

CSSStyleSheet::insertRule should respect the default namespace This patch fixes CSSStyleSheet::insertRule to respect the default namespace a stylesheet. The default namespace is currently only stored during initial parsing of a stylesheet, so this moves the value to be stored on the StyleSheetContents. This is stored separately to the rest of the namespaces to avoid an extra hash map lookup, but could probably be added to the m_namespaces map keyed off a null AtomicString. The added test passes in FF and IE, but fails in Safari. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=201712

Patch Set 1 #

Patch Set 2 : #

Total comments: 1

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -24 lines) Patch
A LayoutTests/fast/css/namespaces/default-namespace-insert-rule.html View 1 2 1 chunk +21 lines, -0 lines 0 comments Download
M Source/core/css/StyleSheetContents.h View 2 chunks +2 lines, -0 lines 0 comments Download
M Source/core/css/StyleSheetContents.cpp View 3 chunks +6 lines, -1 line 0 comments Download
M Source/core/css/parser/CSSParser.cpp View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/css/parser/CSSParserImpl.h View 2 chunks +1 line, -2 lines 0 comments Download
M Source/core/css/parser/CSSParserImpl.cpp View 5 chunks +3 lines, -6 lines 0 comments Download
M Source/core/css/parser/CSSSelectorParser.h View 3 chunks +3 lines, -3 lines 0 comments Download
M Source/core/css/parser/CSSSelectorParser.cpp View 4 chunks +14 lines, -8 lines 0 comments Download
M Source/core/css/parser/CSSSelectorParserTest.cpp View 1 2 chunks +5 lines, -3 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 8 (3 generated)
Timothy Loh
I noticed this was broken while looking over https://codereview.chromium.org/1321943002...
5 years, 3 months ago (2015-09-03 05:42:38 UTC) #2
Timothy Loh
I noticed this was broken while looking over https://codereview.chromium.org/1321943002...
5 years, 3 months ago (2015-09-03 05:42:38 UTC) #3
alancutter (OOO until 2018)
lgtm with nit. https://codereview.chromium.org/1322543004/diff/20001/LayoutTests/fast/css/namespaces/default-namespace-insert-rule.html File LayoutTests/fast/css/namespaces/default-namespace-insert-rule.html (right): https://codereview.chromium.org/1322543004/diff/20001/LayoutTests/fast/css/namespaces/default-namespace-insert-rule.html#newcode12 LayoutTests/fast/css/namespaces/default-namespace-insert-rule.html:12: <div id=div></div> HTML element tagnames as ...
5 years, 3 months ago (2015-09-03 07:40:12 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1322543004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1322543004/40001
5 years, 3 months ago (2015-09-03 07:44:42 UTC) #7
commit-bot: I haz the power
5 years, 3 months ago (2015-09-03 08:47:02 UTC) #8
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=201712

Powered by Google App Engine
This is Rietveld 408576698