Closed Bug 679320 Opened 9 years ago Closed 9 years ago

Add currentURI as a property to the xul:editor element

Categories

(Core :: XUL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

References

Details

(Whiteboard: [qa-])

Attachments

(2 files, 1 obsolete file)

The printUtils code expects that an element passed to it will have a currentURI property, at the moment xul:browser does but xul:editor doesn't and is only accessible via webNavigation or docShell. To make the xul:editor element more like an xul:browser element we need to add a currentURI property to it.

This patch:
* Adds currentURI property to editor.xml using this.webNavigation.currentURI (same as browser.xml does).
Attachment #553438 - Flags: review?(ehsan)
Comment on attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

r=me, but this does need a test too.
Attachment #553438 - Flags: review?(ehsan) → review+
Attached patch test editor (obsolete) — Splinter Review
This patch:
* Adds a test that currentURI property exists.
* Adds a test that currentURI.spec is set to the expected value.
Attachment #553925 - Flags: review?(ehsan)
This time with file hg added!
Attachment #553925 - Attachment is obsolete: true
Attachment #553925 - Flags: review?(ehsan)
Attachment #553927 - Flags: review?(ehsan)
Comment on attachment 553927 [details] [diff] [review]
test editor with file added [Checked in: Comment 9]

>diff --git a/toolkit/content/tests/widgets/Makefile.in b/toolkit/content/tests/widgets/Makefile.in
>+_CHROME_TEST_FILES = \
>+		test_editor.xul \

Rename this to something more specific, like test_editor_currentURI.xul please?

Also, I think you should just add this to _CHROME_FILES.

>diff --git a/toolkit/content/tests/widgets/test_editor.xul b/toolkit/content/tests/widgets/test_editor.xul
>+    try {
>+      is(editor.currentURI.spec, "about:blank",
>+         "currentURI.spec is about:blank");
>+    } catch(ex) {}

Remove the try/catch, please.  If retrieving editor.currentURI.spec fails, we want the test to fail (by a timeout), not silently pass).

r=me with the above.
Attachment #553927 - Flags: review?(ehsan) → review+
Comment on attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/3976958e0e32
Attachment #553438 - Attachment description: currentURI to editor.xml → currentURI to editor.xml [Checked in: Comment 5]
Comment on attachment 553927 [details] [diff] [review]
test editor with file added [Checked in: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/8a0944d82364
Attachment #553927 - Attachment description: test editor with file added → test editor with file added [Checked in: Comment 6]
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Backed out in http://hg.mozilla.org/mozilla-central/rev/0423f32a8054 - despite my initial belief that I just needed to expand on bug 679924 to include a few more test failures, in the end it looks like this made a bunch of the test_reftests_with_caret.html tests permaorange on Windows.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Flags: in-testsuite+
Target Milestone: mozilla9 → ---
Probably mistakenly blamed this, looks more like it must have been the push before it.
Comment on attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/b904a9d949d6
Attachment #553438 - Attachment description: currentURI to editor.xml [Checked in: Comment 5] → currentURI to editor.xml [Checked in: Comment 9]
Attachment #553927 - Attachment description: test editor with file added [Checked in: Comment 6] → test editor with file added [Checked in: Comment 9]
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla9
Is there anything for QA to verify on this bug?
Whiteboard: [qa?]
[Triage Comment]
Please provide STR here by tomorrow at 12:00PM PT 12/14. We will be holding our FF9 sign-offs later that afternoon, and need to be able to verify. Thanks!
Oh, *that's* why the full weight and might of QA is coming down on this ancient bug. It wasn't fixed in the status-firefox9 sense, where it landed on that branch after it was branched and needs to be QAed, it was just like thousands of other things that landed on mozilla-central while now-9 was there.

Your STR are very probably something like "Open a build of SeaMonkey from the same branch as Fx9, open the Composer, type Hello, File, Print Preview," so count your lucky stars, you don't need to do anything here.
Whiteboard: [qa?]
Does this bug only affect Seamonkey and not Firefox? If so, I'm inclined to say QA wont need to spend time verifying the fix.
The xul:editor element, so SeaMonkey's composer, SeaMonkey and Thunderbird's email composer, and hypothetical Firefox extensions that do editing in one particular way, and want to have print and print preview work from their extension editor.

And it has an automated test. And it landed on the trunk.

The only difference between this bug and, say, bug 682338 which you wisely stayed well away from, is that IanN mistakenly thought he was supposed to set the status-firefox9 flag while landing something on the trunk. This is not one of the 36 things with status-firefox9 == fixed that you have to look at, which is why I removed that flag; this is one of the 747 things with target milestone == mozilla9 and resolved (rather than verified) fixed that nobody has and nobody will and nobody needs to QA. Rejoice, and on to your next one!
Thanks Phil, I'm going to mark this qa- as we don't need to spend time verifying this fix.
Whiteboard: [qa-]
Moving to Core:XUL per https://bugzilla.mozilla.org/show_bug.cgi?id=1455336
Component: XP Toolkit/Widgets: XUL → XUL
You need to log in before you can comment on or make changes to this bug.