Move dir attribute from HTMLDocument to Document to match the spec http://www.w3.org/html/wg/drafts/html/master/dom.html#dom-dir We should consider merging blink merge https://src.chromium.org/viewvc/blink?revision=171529&view=revision
Created attachment 234556 [details] Patch proposed patch
Created attachment 234557 [details] Patch proposed patch
Created attachment 234615 [details] Patch Fixed Mac build failure
Please wait for approval from timothy@apple.com (or another member of the Apple Safari Team) before submitting because this patch contains changes to the Apple Mac WebKit.framework public API.
Comment on attachment 234615 [details] Patch Attachment 234615 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5161605872484352 New failing tests: fast/dom/assign-to-prototype-accessor-on-prototype-should-be-silent.html
Created attachment 234620 [details] Archive of layout-test-results from webkit-ews-15 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-15 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
Comment on attachment 234615 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234615&action=review > Source/WebCore/ChangeLog:6 > + Reviewed by NOBODY (OOPS!). You need a blank line after this. > Source/WebCore/ChangeLog:8 > + http://www.w3.org/html/wg/drafts/html/master/dom.html#dom-dir Please refer to a specific version of the spec since WG could be updated in the future. > Source/WebCore/ChangeLog:9 > + This is a blinkMerge from https://src.chromium.org/viewvc/blink?revision=171529&view=revision should read "a blink merge". > Source/WebCore/bindings/objc/PublicDOMInterfaces.h:112 > - (DOMElement *)createElement:(NSString *)tagName; This attribute is certainly not available in OS X 10.5. r- because of this. > Source/WebCore/bindings/objc/PublicDOMInterfaces.h:-456 > -@property (copy) NSString *dir WEBKIT_AVAILABLE_MAC(10_5); I don't think we can make this change safely. > LayoutTests/fast/dom/dir-non-html-document.html:12 > +var xmlDocument = document.implementation.createDocument("", "", null); > +shouldBe("xmlDocument.__proto__", "Document.prototype"); Please test a SVG document as well as a MathML document to ensure assigning a value doesn't modify SVG and/or MathML document. You also need to fix fast/dom/assign-to-prototype-accessor-on-prototype-should-be-silent.html as it assumes that Document.prototype.dir doesn't exist but the change set http://trac.webkit.org/r168385 makes me think that this patch may not be Web compatible to begin with.
Comment on attachment 234615 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=234615&action=review >> Source/WebCore/bindings/objc/PublicDOMInterfaces.h:112 >> - (DOMElement *)createElement:(NSString *)tagName; > > This attribute is certainly not available in OS X 10.5. r- because of this. Yes, this is a bit disingenuous as it documents that this API was available on the base class since OS X 10.5. What is the preferred way is to resolve this issue? I mean, we're moving an API that was made available in OS X 10.5 from a derived class to a base class. I guess you could move this property to the base class mark it as available in a later release and then add a custom setter and getter for dir in DOMHTMLDocument with the WEBKIT_AVAILABLE_MAC(10_5) decoration that forwards to the corresponding setter and getter for the base class property :\
(In reply to comment #8) > (From update of attachment 234615 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=234615&action=review > > >> Source/WebCore/bindings/objc/PublicDOMInterfaces.h:112 > >> - (DOMElement *)createElement:(NSString *)tagName; > > > > This attribute is certainly not available in OS X 10.5. r- because of this. > > [...] What is the preferred way is to resolve this issue? > [...] For completeness, a similar approach to moving a public API from a derived interface to its base interface was done in the patch for <https://bugs.webkit.org/show_bug.cgi?id=131079> and <https://bugs.webkit.org/show_bug.cgi?id=67651>.
Created attachment 235048 [details] Patch Modified as per review comment
If Prashant is not available to land the patch, I believe one should be able to carry over his initial work. Matching the spec is always good.
I took test case from the attached patch but from Google Chrome source and then changed into this JSFiddle: Link - https://jsfiddle.net/jdxf9u4v/show All browsers (Firefox Nightly 105, Chrome Canary 106 and Safari 15.6 on macOS 12.5) match with each other. I am marking this as "RESOLVED CONFIGURATION CHANGED". Please reopen, if I am wrong. Thanks!
<rdar://problem/98197914>