Need to rename DOMURL to URL as per specs.
This will fix the failing test cases in global-constructors.html
Created attachment 126289 [details] Patch
Comment on attachment 126289 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126289&action=review > Source/WebCore/page/DOMWindow.idl:785 > + attribute [Conditional=BLOB] URLConstructor URL; I think we should keep this as webkitURL for a little longer. We are not even close to implementing the whole thing so and there is no rush to rename this. > Source/WebCore/workers/WorkerContext.idl:104 > + attribute [Conditional=BLOB] URLConstructor URL; Keep as webkitURL for now
(In reply to comment #3) > (From update of attachment 126289 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=126289&action=review > > > Source/WebCore/page/DOMWindow.idl:785 > > + attribute [Conditional=BLOB] URLConstructor URL; > > I think we should keep this as webkitURL for a little longer. We are not even close to implementing the whole thing so and there is no rush to rename this. > I agree, but just to make it compatible with other browsers. Currently, I saw all sites use - "window.webkitURL || window.URL" To avoid this I thought to remove prefix webkit and made it as per specs requirement. > > Source/WebCore/workers/WorkerContext.idl:104 > > + attribute [Conditional=BLOB] URLConstructor URL; > > Keep as webkitURL for now Surely, guess we can make it later. Will upload it with webkitURL
Created attachment 126450 [details] Updated Patch
Created attachment 126458 [details] Updated Patch I had missed new changes in IDL files from Haraken. Fixed those.
Based on Arv's comments, lets do this later.
Comment on attachment 126458 [details] Updated Patch Cleared review? from attachment 126458 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
(In reply to comment #7) > Based on Arv's comments, lets do this later. I think what Arv meant here was to keep java script binding side name as webkitURL and change the DOMURLConstructor to URLConstructor. So that when implementation is ready just changing the two lines in DOMWindow.idl & WorkerContext.idl would migrate from JavaScript side later. I need to push big chunk of URL decomposition attributes. But for that it would be better if this DOMURL to URL change goes in. correct me if wrong. Thanks.
We likely want to continue to call the implementation DOMURL so as to be distinct from URL, which is what we plan to rename KURL to.
Sorry for letting this fall off my radar. Keeping the C++ name DOMURL seems better but we should make sure that we only expose the name "URL" to the web page. I think we should name the constructor webkitURL until we at least have all the methods implemented.
Reopening as per Erik's comment. Should I provide the patch only to rename DOMURLConstructor to URLConstructor and migrate DOMURL to URL in WebIDL files?
(In reply to comment #12) > Reopening as per Erik's comment. Should I provide the patch only to rename DOMURLConstructor to URLConstructor and migrate DOMURL to URL in WebIDL files? Yes please.
I'm working on the renaming of the bindings in bug 78721. Once that is done it should be trivial to fix this bug.
(In reply to comment #14) > I'm working on the renaming of the bindings in bug 78721. Once that is done it should be trivial to fix this bug. Great. That would really help to fix this. I was wondering since last week how to fix this without changing file names. Now I hope it will be easy after your patch :)
Created attachment 128672 [details] Updated Patch Updated patch for Renaming DOMURL to URL using InterfaceName. Can you review once?
Comment on attachment 128672 [details] Updated Patch Can you see if some of the tests can be updated? View in context: https://bugs.webkit.org/attachment.cgi?id=128672&action=review > LayoutTests/platform/gtk/fast/dom/prototype-inheritance-2-expected.txt:234 > +PASS Window from inner > +PASS WindowConstructor from inner.document.forms.testForm.0.ownerDocument.defaultView.constructor > +PASS WindowPrototype from inner.document.forms.testForm.0.ownerDocument.defaultView.__proto__ Please rebaseline > LayoutTests/platform/mac/fast/js/constructor-length-expected.txt:31 > +FAIL URL.length should be 0. Threw exception ReferenceError: Can't find variable: URL Please fix this > LayoutTests/platform/mac/fast/js/global-constructors-expected.txt:340 > +FAIL webkitURL.toString() should be [object webkitURLConstructor]. Was [object URLConstructor]. Not much to do about this one until we strip the prefix.
Created attachment 129003 [details] Updated Patch Fixed test cases. Rebaslined. Eric/Adam can you review once?
Comment on attachment 129003 [details] Updated Patch Rejecting attachment 129003 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2 Last 500 characters of output: saving rejects to file LayoutTests/platform/mac/fast/js/constructor-length-expected.txt.rej patching file LayoutTests/platform/mac/fast/js/global-constructors-expected.txt patching file LayoutTests/platform/qt/fast/dom/prototype-inheritance-2-expected.txt patching file LayoutTests/platform/qt/fast/js/global-constructors-expected.txt Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Adam Barth']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue/ Full output: http://queues.webkit.org/results/11757822
Created attachment 129841 [details] Rebaseline Patch Patch need to be rebaselined. Updated.
Comment on attachment 129841 [details] Rebaseline Patch The rebaselining looks OK. Based on abarth's previous r+, let me r+ it.
Comment on attachment 129841 [details] Rebaseline Patch Clearing flags on attachment: 129841 Committed r109574: <http://trac.webkit.org/changeset/109574>
All reviewed patches have been landed. Closing bug.