Last Comment Bug 679320 - Add currentURI as a property to the xul:editor element
: Add currentURI as a property to the xul:editor element
Status: RESOLVED FIXED
[qa-]
:
Product: Core
Classification: Components
Component: XP Toolkit/Widgets: XUL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Ian Neal
:
Mentors:
Depends on:
Blocks: 21432
  Show dependency treegraph
 
Reported: 2011-08-16 04:38 PDT by Ian Neal
Modified: 2011-12-13 19:11 PST (History)
5 users (show)
iann_bugzilla: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
currentURI to editor.xml [Checked in: Comment 9] (1.30 KB, patch)
2011-08-16 04:38 PDT, Ian Neal
ehsan: review+
Details | Diff | Splinter Review
test editor (937 bytes, patch)
2011-08-17 15:23 PDT, Ian Neal
no flags Details | Diff | Splinter Review
test editor with file added [Checked in: Comment 9] (2.56 KB, patch)
2011-08-17 15:24 PDT, Ian Neal
ehsan: review+
Details | Diff | Splinter Review

Description Ian Neal 2011-08-16 04:38:52 PDT
Created attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

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).
Comment 1 :Ehsan Akhgari 2011-08-17 04:56:25 PDT
Comment on attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

r=me, but this does need a test too.
Comment 2 Ian Neal 2011-08-17 15:23:11 PDT
Created attachment 553925 [details] [diff] [review]
test editor

This patch:
* Adds a test that currentURI property exists.
* Adds a test that currentURI.spec is set to the expected value.
Comment 3 Ian Neal 2011-08-17 15:24:52 PDT
Created attachment 553927 [details] [diff] [review]
test editor with file added [Checked in: Comment 9]

This time with file hg added!
Comment 4 :Ehsan Akhgari 2011-08-17 15:31:00 PDT
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.
Comment 5 Ian Neal 2011-08-17 16:16:56 PDT
Comment on attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/3976958e0e32
Comment 6 Ian Neal 2011-08-17 16:17:20 PDT
Comment on attachment 553927 [details] [diff] [review]
test editor with file added [Checked in: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/8a0944d82364
Comment 7 Phil Ringnalda (:philor, back in August) 2011-08-17 22:06:19 PDT
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.
Comment 8 Phil Ringnalda (:philor, back in August) 2011-08-17 23:54:04 PDT
Probably mistakenly blamed this, looks more like it must have been the push before it.
Comment 9 Ian Neal 2011-08-18 09:30:00 PDT
Comment on attachment 553438 [details] [diff] [review]
currentURI to editor.xml [Checked in: Comment 9]

http://hg.mozilla.org/mozilla-central/rev/b904a9d949d6
Comment 10 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-11-21 16:41:09 PST
Is there anything for QA to verify on this bug?
Comment 11 Alex Keybl [:akeybl] 2011-12-13 16:09:50 PST
[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!
Comment 12 Phil Ringnalda (:philor, back in August) 2011-12-13 16:31:41 PST
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.
Comment 13 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-13 16:47:51 PST
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.
Comment 14 Phil Ringnalda (:philor, back in August) 2011-12-13 18:07:41 PST
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!
Comment 15 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2011-12-13 19:11:41 PST
Thanks Phil, I'm going to mark this qa- as we don't need to spend time verifying this fix.

Note You need to log in before you can comment on or make changes to this bug.