Closed Bug 1583348 Opened 6 years ago Closed 6 years ago

Write a gTest for mozilla::dom::DOMParser::CreateWithoutGlobal()

Categories

(Core :: DOM: Core & HTML, task)

Product:
Core
Component:
DOM: Core & HTML
Type:
task
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox71 --- fixed

People

(Reporter: jorgk-bmo, Assigned: jorgk-bmo)

Details

Attachments

(1 file, 2 obsolete files)

M-C version: 1583348-gtest-parser.patch (v2)
2.88 KB, patch
bzbarsky
: review+
Details | Diff | Splinter Review

M-C provides the mozilla::dom::DOMParser::CreateWithoutGlobal() function for us, but it's not tested anywhere, see bug 1454842 comment #13.

TB's processing of HTML mail relies on that function, see:
https://searchfox.org/comm-central/rev/30f32c40a88dc34746d22697dad8fe99c7922054/mailnews/mime/src/mimeTextHTMLParsed.cpp#83

I'm sure other tests would pick up a failure, but we should also test the function directly now that we have gTests in automation.

Comment on attachment 9099107 [details] [diff] [review] C-C version: 1583348-gtest-parser.patch Let's see whether we can stick the test into M-C. I'll prepare a patch for that.
Attachment #9099107 - Flags: review?(acelists)
Attachment #9099107 - Attachment description: 1583348-gtest-parser.patch → C-C version: 1583348-gtest-parser.patch

Hello Boris, this is a follow-up to bug 1454842 comment #11. Can we implement this test in M-C so we notice a problem before it's committed? mach gtest TestParser* runs the test successfully. Please let me know which changes you require. Or whether you prefer not to have it in M-C.

Attachment #9099234 - Flags: review?(bzbarsky)
Comment on attachment 9099234 [details] [diff] [review] M-C version: 1583348-gtest-parser.patch >+ nsString htmlInput; >+ htmlInput.AssignLiteral( >+ "<html><head>" >+ "<meta http-equiv=\"content-type\" content=\"text/html; charset=\">" >+ "</head><body>Hello <b>Thunderbird!</b></body></html>"); Any reason this is not NS_NAMED_LITERAL_STRING? >+ RefPtr<mozilla::dom::DOMParser> parser = >+ mozilla::dom::DOMParser::CreateWithoutGlobal(rv2); If rv2 here fails, something needs to happen. Probably break? >+ nsCOMPtr<mozilla::dom::Document> document = parser->ParseFromString( >+ htmlInput, mozilla::dom::SupportedType::Text_html, rv2); >+ if (rv2.Failed()) break; rv2 should be an IgnoredErrorResult; otherwise this will assert if the failure path is taken. >+ printf("all tests passed\n"); Why?
Attachment #9099234 - Flags: review?(bzbarsky) → review+

Thanks, Boris, could you stick the r+ onto it one more time.
EDIT: https://bugzilla.mozilla.org/attachment.cgi?oldid=9099234&action=interdiff&newid=9099710&headers=1

The answer to all your questions is possibly lack of experience.

mach gtest TestParser* still passes.

Attachment #9099107 - Attachment is obsolete: true
Attachment #9099234 - Attachment is obsolete: true
Attachment #9099710 - Flags: review?(bzbarsky)
Attachment #9099710 - Flags: review?(bzbarsky) → review+

Dear sheriff, can you please land the attached patch the old-fashioned way.

Please import the patch with
hg qimport bz:1583348
or
hg qimport https://bugzilla.mozilla.org/attachment.cgi?id=9099710 -n 1583348.patch
to avoid changing the author.

Component: General → DOM: Core & HTML
Keywords: checkin-needed
Product: MailNews Core → Core
Version: unspecified → Trunk

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ec88fbbfcf8
Implement gTest for mozilla::dom::DOMParser::CreateWithoutGlobal(). r=bz

Keywords: checkin-needed

https://hg.mozilla.org/mozilla-central/rev/7ec88fbbfcf8

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox71: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: