Move DOMString back to script #11326

nox · 2016-05-22T11:35:51Z

This change is

highfive · 2016-05-22T11:35:54Z

Heads up! This PR modifies the following files:

@bholley: components/style/attr.rs
@KiChjang: components/script/dom/workerlocation.rs, components/script/dom/textencoder.rs, components/script/dom/file.rs, components/script/dom/xmldocument.rs, components/script/dom/htmlbrelement.rs, components/script/dom/focusevent.rs, components/script/dom/navigatorinfo.rs, components/script/dom/htmlcanvaselement.rs, components/script/dom/blob.rs, components/script/textinput.rs, components/script/dom/location.rs, components/script/dom/eventtarget.rs, components/script/timers.rs, components/script/dom/htmlfieldsetelement.rs, components/script/dom/htmlformcontrolscollection.rs, components/script/dom/workerglobalscope.rs, components/script/dom/pluginarray.rs, components/script/dom/htmllielement.rs, components/script/dom/htmlpreelement.rs, components/script/dom/domstringmap.rs, components/script/dom/virtualmethods.rs, components/script/dom/htmlheadelement.rs, components/script/dom/create.rs, components/script/dom/htmlmapelement.rs, components/script/dom/htmldetailselement.rs, components/script/dom/urlsearchparams.rs, components/script/dom/htmldialogelement.rs, components/script/dom/htmloptgroupelement.rs, components/script/dom/htmlimageelement.rs, components/script/script_thread.rs, components/script/dom/console.rs, components/script/dom/css.rs, components/script/dom/worker.rs, components/script/dom/attr.rs, components/script/dom/htmlbuttonelement.rs, components/script/dom/node.rs, components/script/dom/documentfragment.rs, components/script/dom/event.rs, components/script/dom/radionodelist.rs, components/script/dom/webglshader.rs, components/script/dom/htmldataelement.rs, components/script/dom/htmlanchorelement.rs, components/script/dom/url.rs, components/script/dom/workernavigator.rs, components/script/dom/canvasgradient.rs, components/script/dom/htmltableelement.rs, components/script/dom/plugin.rs, components/script/dom/progressevent.rs, components/script/dom/textdecoder.rs, components/script/dom/htmldatalistelement.rs, components/script/dom/keyboardevent.rs, components/script/dom/mouseevent.rs, components/script/dom/activation.rs, components/script/dom/range.rs, components/script/dom/testbindingproxy.rs, components/script/dom/htmlulistelement.rs, components/script/dom/htmlselectelement.rs, components/script/dom/pagetransitionevent.rs, components/script/dom/bluetoothuuid.rs, components/script/dom/htmldlistelement.rs, components/script/dom/processinginstruction.rs, components/script/dom/bluetoothdevice.rs, components/script/dom/htmlparamelement.rs, components/script/devtools.rs, components/script/dom/htmlvideoelement.rs, components/script/dom/htmllinkelement.rs, components/script/dom/testbinding.rs, components/script/dom/cssstyledeclaration.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/browsingcontext.rs, components/script/dom/messageevent.rs, components/script/dom/webglcontextevent.rs, components/script/dom/xmlhttprequest.rs, components/script/dom/htmlmeterelement.rs, components/script/dom/htmlmediaelement.rs, components/script/dom/customevent.rs, components/script/dom/htmltablecellelement.rs, components/script/dom/window.rs, components/script/dom/uievent.rs, components/script/webdriver_handlers.rs, components/script/dom/htmlmodelement.rs, components/script/dom/stylesheet.rs, components/script/dom/namednodemap.rs, components/script/dom/htmlinputelement.rs, components/script/dom/bindings/str.rs, components/script/dom/domimplementation.rs, components/script/dom/htmlareaelement.rs, components/script/parse/html.rs, components/script/dom/htmlsourceelement.rs, components/script/dom/htmltrackelement.rs, components/script/dom/htmlframeelement.rs, components/script/dom/htmlheadingelement.rs, components/script/dom/webglprogram.rs, components/script/dom/bindings/conversions.rs, components/script/dom/htmlolistelement.rs, components/script/dom/bindings/trace.rs, components/script/dom/eventsource.rs, components/script/dom/closeevent.rs, components/script/dom/domexception.rs, components/script/dom/htmlhrelement.rs, components/script/dom/htmlprogresselement.rs, components/script/dom/htmltablecolelement.rs, components/script/dom/bluetoothremotegattdescriptor.rs, components/script/dom/htmlcollection.rs, components/script/dom/bindings/utils.rs, components/script/dom/userscripts.rs, components/script/dom/htmlmetaelement.rs, components/script/dom/filereader.rs, components/script/dom/beforeunloadevent.rs, components/script/dom/htmltabledatacellelement.rs, components/script/dom/hashchangeevent.rs, components/script/dom/htmltitleelement.rs, components/script/dom/bluetoothremotegattserver.rs, components/script/dom/document.rs, components/script/dom/htmllegendelement.rs, components/script/dom/htmltableheadercellelement.rs, components/script/dom/element.rs, components/script/dom/htmlparagraphelement.rs, components/script/dom/popstateevent.rs, components/script/dom/htmlspanelement.rs, components/script/dom/htmltemplateelement.rs, components/script/dom/htmllabelelement.rs, components/script/dom/bindings/codegen/CodegenRust.py, components/script/dom/htmlhtmlelement.rs, components/script/dom/text.rs, components/script/parse/xml.rs, components/script/dom/bindings/xmlname.rs, components/script/dom/htmlstyleelement.rs, components/script/dom/bluetoothremotegattservice.rs, components/script/dom/htmltimeelement.rs, components/script/dom/htmldirectoryelement.rs, components/script/dom/storage.rs, components/script/dom/canvasrenderingcontext2d.rs, components/script/dom/characterdata.rs, components/script/dom/webglactiveinfo.rs, components/script/dom/htmlfontelement.rs, components/script/dom/domtokenlist.rs, components/script/dom/htmlframesetelement.rs, components/script/dom/storageevent.rs, components/script/dom/htmlappletelement.rs, components/script/dom/htmltextareaelement.rs, components/script/dom/mimetypearray.rs, components/script/dom/navigator.rs, components/script/dom/htmlbaseelement.rs, components/script/dom/htmlquoteelement.rs, components/script/dom/htmlscriptelement.rs, components/script/dom/htmlaudioelement.rs, components/script/dom/documenttype.rs, components/script/dom/mimetype.rs, components/script/dom/htmltablesectionelement.rs, components/script/dom/macros.rs, components/script/dom/htmlunknownelement.rs, components/script/dom/htmltablerowelement.rs, components/script/dom/htmloptionelement.rs, components/script/dom/htmlobjectelement.rs, components/script/dom/forcetouchevent.rs, components/script/dom/comment.rs, components/script/dom/htmlembedelement.rs, components/script/dom/htmloutputelement.rs, components/script/lib.rs, components/script/dom/errorevent.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/script/dom/touchevent.rs, components/script/dom/domparser.rs, components/script/dom/bluetooth.rs, components/script/dom/htmldivelement.rs, components/script/dom/bluetoothremotegattcharacteristic.rs, components/script/dom/htmlbodyelement.rs, components/script/dom/htmliframeelement.rs, components/script/dom/htmlelement.rs, components/script/dom/htmlformelement.rs, components/script/dom/websocket.rs, components/script/dom/htmltablecaptionelement.rs, components/script/dom/formdata.rs
@emilio: components/script/dom/webglshader.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/webglcontextevent.rs, components/script/dom/webglprogram.rs, components/script/dom/webglactiveinfo.rs

highfive · 2016-05-22T11:35:55Z

Warning

These commits modify unsafe code. Please review it carefully!

nox · 2016-05-22T11:36:04Z

@bors-servo try

r? @SimonSapin


        Auto merge of #11326 - nox:non-geckolib, r=<try>

Move DOMString back to script

bors-servo · 2016-05-22T11:36:08Z

⌛ Trying commit c9fb286 with merge ac4eafc...

bors-servo · 2016-05-22T11:49:55Z

💔 Test failed - linux-dev

highfive · 2016-05-22T11:51:08Z

New code was committed to pull request.

jdm · 2016-05-22T13:26:41Z

Why all the changes from DOMString to String?

nox · 2016-05-22T13:28:11Z

Because AttrValue using DOMString is the reason why we have that non-geckolib mess, and why style ends up transitively depending on js through util in the normal build. This is what this patch removes.

jdm · 2016-05-22T14:43:51Z

It makes it more difficult to experiment with different underlying representations in the future, right?

nox · 2016-05-22T14:45:24Z

Given we share the style crate with geckolib, we are going to need something other than DOMString in AttrValue if we want to get fancy with string representations anyway.

Ms2ger · 2016-05-23T07:25:15Z

@bholley how does this work in Geckolib? Do you use restyle_hints.rs at all?

bors-servo · 2016-05-23T09:15:38Z

☔ The latest upstream changes (presumably #11225) made this pull request unmergeable. Please resolve the merge conflicts.

highfive · 2016-05-23T09:32:12Z

New code was committed to pull request.

nox · 2016-05-23T09:32:28Z

r? @Ms2ger

bholley · 2016-05-23T20:37:53Z

@bholley how does this work in Geckolib?

For non-atomized strings we're currently converting. Depending on the profiles I may end up needing to introduce a wrapper class for certain usages of string within components/style that just acts like String in servo builds and does some tricky utf-16 stuff in Gecko builds. DOMString might help with building that layer, but I can't say so concretely enough to justify objecting to these changes on those grounds.

Do you use restyle_hints.rs at all?
We don't yet but we will.


        Move DOMString back to script

This entirely removes the 'non-geckolib' feature of the util crate.

highfive · 2016-05-24T08:55:19Z

New code was committed to pull request.

Ms2ger · 2016-05-24T08:56:27Z

@bors-servo r+

I suppose.

bors-servo · 2016-05-24T08:56:28Z

📌 Commit cdc7bca has been approved by Ms2ger

nox · 2016-05-24T09:05:44Z

@bors-servo p=1

bors-servo · 2016-05-24T09:07:31Z

⌛ Testing commit cdc7bca with merge 6abcd79...


        Auto merge of #11326 - nox:non-geckolib, r=Ms2ger

Move DOMString back to script  This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11326)

bors-servo · 2016-05-24T09:58:32Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

nox added A-content/bindings I-cleanup labels May 22, 2016

highfive assigned jdm May 22, 2016

highfive added the S-awaiting-review label May 22, 2016

highfive assigned SimonSapin and unassigned jdm May 22, 2016

bors-servo added a commit that referenced this pull request May 22, 2016

Auto merge of #11326 - nox:non-geckolib, r=<try>

ac4eafc

Move DOMString back to script

highfive added the S-tests-failed label May 22, 2016

nox force-pushed the nox:non-geckolib branch from c9fb286 to 80f07e4 May 22, 2016

highfive removed the S-tests-failed label May 22, 2016

highfive added the S-needs-rebase label May 23, 2016

nox removed the S-needs-rebase label May 23, 2016

nox force-pushed the nox:non-geckolib branch from 80f07e4 to 165e6d2 May 23, 2016

highfive assigned Ms2ger and unassigned SimonSapin May 23, 2016

Move DOMString back to script

Loading status checks…

cdc7bca

This entirely removes the 'non-geckolib' feature of the util crate.

nox force-pushed the nox:non-geckolib branch from 165e6d2 to cdc7bca May 24, 2016

bors-servo merged commit cdc7bca into servo:master May 24, 2016
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

nox deleted the nox:non-geckolib branch May 24, 2016

servo / servo

Move DOMString back to script #11326

Move DOMString back to script #11326

nox commented May 22, 2016 •

edited by larsbergstrom

highfive commented May 22, 2016

highfive commented May 22, 2016

nox commented May 22, 2016

bors-servo commented May 22, 2016

bors-servo commented May 22, 2016

highfive commented May 22, 2016

jdm commented May 22, 2016

nox commented May 22, 2016

jdm commented May 22, 2016

nox commented May 22, 2016

Ms2ger commented May 23, 2016

bors-servo commented May 23, 2016

highfive commented May 23, 2016

nox commented May 23, 2016

bholley commented May 23, 2016

highfive commented May 24, 2016

Ms2ger commented May 24, 2016

bors-servo commented May 24, 2016

nox commented May 24, 2016

bors-servo commented May 24, 2016

bors-servo commented May 24, 2016

servo / servo

Join GitHub today

Move DOMString back to script #11326

Move DOMString back to script #11326

Conversation

nox commented May 22, 2016 • edited by larsbergstrom

highfive commented May 22, 2016

highfive commented May 22, 2016

nox commented May 22, 2016

bors-servo commented May 22, 2016

bors-servo commented May 22, 2016

highfive commented May 22, 2016

jdm commented May 22, 2016

nox commented May 22, 2016

jdm commented May 22, 2016

nox commented May 22, 2016

Ms2ger commented May 23, 2016

bors-servo commented May 23, 2016

highfive commented May 23, 2016

nox commented May 23, 2016

bholley commented May 23, 2016

highfive commented May 24, 2016

Ms2ger commented May 24, 2016

bors-servo commented May 24, 2016

nox commented May 24, 2016

bors-servo commented May 24, 2016

bors-servo commented May 24, 2016

nox commented May 22, 2016 •

edited by larsbergstrom