Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement Node.{lookupPrefix, lookupNamespaceURI, isDefaultNamespace} #1826

Closed
Ms2ger opened this issue Mar 5, 2014 · 15 comments
Closed

Implement Node.{lookupPrefix, lookupNamespaceURI, isDefaultNamespace} #1826

Ms2ger opened this issue Mar 5, 2014 · 15 comments

Comments

@Ms2ger
Copy link
Contributor

@Ms2ger Ms2ger commented Mar 5, 2014

Spec at http://dom.spec.whatwg.org/#locate-a-namespace-prefix and below.

Probably wait for #1737 to happen first. Blocks #1655.

@ienzam
Copy link
Contributor

@ienzam ienzam commented Mar 15, 2015

Hi, I will like to work on it.

@jdm
Copy link
Member

@jdm jdm commented Mar 16, 2015

@ienzam Great! Let us know if you have any questions.

@jdm jdm added the C-assigned label Mar 16, 2015
@ienzam
Copy link
Contributor

@ienzam ienzam commented Mar 16, 2015

Two questions

  1. How can I test my code?
    Do I need to write an integration test here - https://github.com/servo/servo/tree/master/tests/content ?
    Or can I write some unit testing somewhere?
  2. Where can I get more explanation about JSRef, root(), r() - any design guide or how to use them? I am mostly doing copy paste from existing code. http://doc.servo.org/ helps a little, but not much :-(
@Ms2ger
Copy link
Contributor Author

@Ms2ger Ms2ger commented Mar 16, 2015

  1. Preferably in https://github.com/w3c/web-platform-tests
  2. If http://doc.servo.org/script/dom/bindings/js/index.html doesn't explain enough, please throw your questions at me so I can improve that.
@ienzam
Copy link
Contributor

@ienzam ienzam commented Mar 21, 2015

Hi, is there a way to test quickly?

My current workflow is:

  1. Added one test file in tests/content
  2. Edit code at components/script/dom/node.rs
  3. Run
    ./mach test-content
  4. Repeat from 2.

It is taking a lot of time for building servo. Is there a faster way?

@jdm
Copy link
Member

@jdm jdm commented Mar 21, 2015

You can run individual content tests, but there's no faster way to rebuild after making changes, unfortunately.

@jdm
Copy link
Member

@jdm jdm commented Mar 21, 2015

./mach test path/to/test/file.html will run the appropriate test harness on the provided file.

@ienzam
Copy link
Contributor

@ienzam ienzam commented Mar 26, 2015

Hi,
I have implemented lookupPrefix - ienzam@3dea397
Can you please review the code and give some feedback, if I am doing anything wrong, or how I can make it more Rust-y?

ienzam added a commit to ienzam/servo that referenced this issue Mar 27, 2015
ienzam added a commit to ienzam/servo that referenced this issue Apr 12, 2015
ienzam added a commit to ienzam/servo that referenced this issue Apr 12, 2015
ienzam added a commit to ienzam/servo that referenced this issue Apr 12, 2015
ienzam added a commit to ienzam/servo that referenced this issue Apr 16, 2015
bors-servo pushed a commit that referenced this issue Apr 16, 2015
Tracking issue #1826 - implemented one of the three methods.

This is follow up of #5402. Rebased and squashed and made the method iterative.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5650)
<!-- Reviewable:end -->
ienzam added a commit to ienzam/servo that referenced this issue Apr 16, 2015
bors-servo pushed a commit that referenced this issue Apr 16, 2015
Tracking issue #1826 - implemented one of the three methods.

This is follow up of #5402. Rebased and squashed and made the method iterative.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5650)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Apr 16, 2015
Tracking issue #1826 - implemented one of the three methods.

This is follow up of #5402. Rebased and squashed and made the method iterative.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5650)
<!-- Reviewable:end -->
bors-servo pushed a commit that referenced this issue Apr 16, 2015
bors-servo
Tracking issue #1826 - implemented one of the three methods.

This is follow up of #5402. Rebased and squashed and made the method iterative.

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/5650)
<!-- Reviewable:end -->
@ienzam
Copy link
Contributor

@ienzam ienzam commented Apr 19, 2015

Hi,
I cannot test the tests now it seems.
I am running both

./mach test-wpt tests/wpt/mozilla/tests/mozilla/node_lookupPrefix.html

and

./mach test-wpt tests/wpt/mozilla/tests/mozilla/node_lookupPrefix.html

But it is not running any tests (and also not building servo)

Summary
=======

Ran 0 tests
Expected results: 0
Unexpected results: 0

OK

What is the new way to build and run tests?

@ienzam
Copy link
Contributor

@ienzam ienzam commented Apr 19, 2015

Found it (autocomplete is gone :( )

./mach test-wpt /_mozilla/mozilla/node_lookupPrefix.html 
@jdm jdm closed this Apr 19, 2015
@ienzam
Copy link
Contributor

@ienzam ienzam commented Apr 20, 2015

@jdm the issue is not fully fixed yet, I only implemented the first method, the other two are still left, working on it.

@jdm jdm reopened this Apr 20, 2015
@jdm
Copy link
Member

@jdm jdm commented Apr 20, 2015

Oops!

@ienzam
Copy link
Contributor

@ienzam ienzam commented Apr 24, 2015

Can you please help me interpret this line (https://dom.spec.whatwg.org/#locate-a-namespace bullet point 2) -

If it (element) has an attribute whose namespace is the XMLNS namespace,
namespace prefix is "xmlns" and local name is (input) prefix,
or if prefix is null and it has an attribute whose namespace is the XMLNS namespace,
namespace prefix is null and local name is "xmlns": 

I am interpreting it as

If the element has an attribute 
whose
namespace is the XMLNS namespace
and
(
  (namespace prefix is "xmlns" and local name is input prefix)
  or
  (input prefix is null and namespace prefix is null and local name is "xmlns")
)

Am I correctly interpreting it?

Also cannot find any test for lookupNamespaceURI.

@Ms2ger
Copy link
Contributor Author

@Ms2ger Ms2ger commented Apr 24, 2015

I read

(If it has an attribute whose
  (namespace is the XMLNS namespace,
   namespace prefix is "xmlns" and
   local name is prefix)), or
(if
  prefix is null and
  it has an attribute whose
    (namespace is the XMLNS namespace,
    namespace prefix is null and
    local name is "xmlns"))

or

If it has an attribute whose namespace is the XMLNS namespace and
(that attribute's namespace prefix is "xmlns" and
 that attribute's local name is prefix), or
(prefix is null and
 that attribute's namespace prefix is null and
 that attribute's local name is "xmlns")
@dzbarsky
Copy link
Member

@dzbarsky dzbarsky commented Jul 5, 2015

I have a patch and some tests for this, just need to clean them up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.